From 724a5b2536850cfc0d34deb21688507c10903060 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 30 Apr 2021 21:09:24 +0530 Subject: [PATCH 01/49] fix: Evaluate boolean values better via /api/resource/ `GET /api/resource/ToDo?limit=10&debug=False&as_dict=0` would be received by the resource handler as debug="False" and as_dict="0" which are both truthy values. So, even though you requested for a list of lists response without debugging on, you'd get the exact opposite; debug on and a list of dicts. - Evaluate boolean values for `GET /api/resource/` - Added `limit` parameter as an alias for `limit_page_length` - Added `frappe.utils.data.sbool` that converts strings to bool values if applicable. - Added some seemingly stupid comments for the sake of consistency. --- frappe/api.py | 47 +++++++++++++++++++++++++++++--------------- frappe/utils/data.py | 20 +++++++++++++++++++ 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/frappe/api.py b/frappe/api.py index 9039ae0e5f..c69f76a755 100644 --- a/frappe/api.py +++ b/frappe/api.py @@ -11,6 +11,7 @@ import frappe.client import frappe.handler from frappe import _ from frappe.utils.response import build_response +from frappe.utils.data import sbool def handle(): @@ -108,25 +109,39 @@ def handle(): elif doctype: if frappe.local.request.method == "GET": - if frappe.local.form_dict.get('fields'): - frappe.local.form_dict['fields'] = json.loads(frappe.local.form_dict['fields']) - frappe.local.form_dict.setdefault('limit_page_length', 20) - frappe.local.response.update({ - "data": frappe.call( - frappe.client.get_list, - doctype, - **frappe.local.form_dict - ) - }) + # set fields for frappe.get_list + if frappe.local.form_dict.get("fields"): + frappe.local.form_dict["fields"] = json.loads(frappe.local.form_dict["fields"]) + + # set limit of records for frappe.get_list + frappe.local.form_dict.setdefault( + "limit_page_length", + frappe.local.form_dict.limit or frappe.local.form_dict.limit_page_length or 20, + ) + + # convert strings to native types + frappe.local.form_dict.update( + {x: sbool(y) for x, y in frappe.local.form_dict.items()} + ) + + # evaluate frappe.get_list + data = frappe.call(frappe.client.get_list, doctype, **frappe.local.form_dict) + + # set frappe.get_list result to response + frappe.local.response.update({"data": data}) if frappe.local.request.method == "POST": + # fetch data from from dict data = get_request_form_data() - data.update({ - "doctype": doctype - }) - frappe.local.response.update({ - "data": frappe.get_doc(data).insert().as_dict() - }) + data.update({"doctype": doctype}) + + # insert document from request data + doc = frappe.get_doc(data).insert() + + # set response data + frappe.local.response.update({"data": doc.as_dict()}) + + # commit for POST requests frappe.db.commit() else: raise frappe.DoesNotExistError diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 3ffa8dc874..9d81aac467 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -622,6 +622,26 @@ def ceil(s): def cstr(s, encoding='utf-8'): return frappe.as_unicode(s, encoding) +def sbool(x): + """Converts str object to Boolean if possible. + Example: + "true" becomes True + "1" becomes True + "{}" remains "{}" + + Args: + x (str): String to be converted to Bool + + Returns: + object: Returns Boolean or type(x) + """ + from distutils.util import strtobool + + try: + return bool(strtobool(x)) + except Exception: + return x + def rounded(num, precision=0): """round method for round halfs to nearest even algorithm aka banker's rounding - compatible with python3""" precision = cint(precision) From b018236d183dfb524afc6d6849562ef906210d4b Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 10 May 2021 11:16:47 +0530 Subject: [PATCH 02/49] fix: Grid row deletion fix --- frappe/public/js/frappe/form/grid.js | 7 ++++++- frappe/public/js/frappe/form/grid_row.js | 7 +++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 4d381c9be7..9b05809aba 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -196,7 +196,9 @@ export default class Grid { tasks.push(() => { if (dirty) { this.refresh(); - this.frm.script_manager.trigger(this.df.fieldname + "_delete", this.doctype); + if (this.frm) { + this.frm.script_manager.trigger(this.df.fieldname + "_delete", this.doctype); + } } }); @@ -345,6 +347,9 @@ export default class Grid { if (d.idx === undefined) { d.idx = ri + 1; } + if (d.name === undefined) { + d.name = "row" + d.idx + } if (this.grid_rows[ri] && !append_row) { var grid_row = this.grid_rows[ri]; grid_row.doc = d; diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index f6da88df57..bbcf0f707b 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -93,8 +93,11 @@ export default class GridRow { } else { data = this.grid.df.data; } - - const index = data.findIndex(d => d.name === me.doc.name); + let index = -1; + + if (me.doc.name) { + index = data.findIndex(d => d.name === me.doc.name); + } if (index > -1) { // mutate array directly, From 6a9b93bfa5d6b5569681c358cc0a2c66351603fa Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 10 May 2021 11:37:48 +0530 Subject: [PATCH 03/49] fix: sider fix --- frappe/public/js/frappe/form/grid.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 9b05809aba..00f9d6abca 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -348,7 +348,7 @@ export default class Grid { d.idx = ri + 1; } if (d.name === undefined) { - d.name = "row" + d.idx + d.name = "row" + d.idx; } if (this.grid_rows[ri] && !append_row) { var grid_row = this.grid_rows[ri]; From c822c54f558383f2f7c92a688d3db8b198aa2bb8 Mon Sep 17 00:00:00 2001 From: hasnain2808 Date: Wed, 12 May 2021 17:03:25 +0530 Subject: [PATCH 04/49] fix: allow updating naming series --- .../document_naming_rule.js | 41 +++++++++++++++++++ .../document_naming_rule.py | 5 +++ 2 files changed, 46 insertions(+) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 56b5c2fdf4..dcb60cd7d5 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -4,6 +4,7 @@ frappe.ui.form.on('Document Naming Rule', { refresh: function(frm) { frm.trigger('document_type'); + frm.trigger("add_button") }, document_type: (frm) => { // update the select field options with fieldnames @@ -20,5 +21,45 @@ frappe.ui.form.on('Document Naming Rule', { ); }); } + }, + add_button: (frm) => { + frm.add_custom_button(__('Update Counter'), function() { + + const fields = [{ + fieldtype: 'Data', + fieldname: 'new_counter', + label: __('New Counter'), + default: frm.doc.counter, + reqd: 1, + description: __('This will update the counter and will affect all documents that will be created') + }] + + let primary_action_label = __('Save'); + + let primary_action = (fields) => { + debugger + frappe.call({ + method: 'frappe.core.doctype.document_naming_rule.document_naming_rule.update_current', + args: { + name: frm.doc.name, + new_counter: fields.new_counter + }, + callback: function() { + frm.set_value("counter", fields.new_counter) + dialog.hide() + } + }) + }; + + var dialog = new frappe.ui.Dialog({ + title: __('Update Counter Value for Prefix: ' + frm.doc.prefix), + fields, + primary_action_label, + primary_action + }); + + dialog.show() + + }); } }); diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 4b34293af6..13d54dffdd 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -30,3 +30,8 @@ class DocumentNamingRule(Document): counter = frappe.db.get_value(self.doctype, self.name, 'counter', for_update=True) or 0 doc.name = self.prefix + ('%0'+str(self.prefix_digits)+'d') % (counter + 1) frappe.db.set_value(self.doctype, self.name, 'counter', counter + 1) + +@frappe.whitelist() +def update_current(name, new_counter): + frappe.db.set_value('Document Naming Rule', name, 'counter', new_counter) + frappe.db.commit() From c246c82a85d8a83fc71193905bf75febef89634a Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Wed, 12 May 2021 17:10:35 +0530 Subject: [PATCH 05/49] fix: remove debugger --- frappe/core/doctype/document_naming_rule/document_naming_rule.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index dcb60cd7d5..5d1540588d 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -37,7 +37,6 @@ frappe.ui.form.on('Document Naming Rule', { let primary_action_label = __('Save'); let primary_action = (fields) => { - debugger frappe.call({ method: 'frappe.core.doctype.document_naming_rule.document_naming_rule.update_current', args: { From bb4be327511eb7f736282fee6eaf5ee6d88ebc7a Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Wed, 12 May 2021 17:15:18 +0530 Subject: [PATCH 06/49] chore: add semicolons --- .../document_naming_rule/document_naming_rule.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 5d1540588d..e837ca2d6c 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -4,7 +4,7 @@ frappe.ui.form.on('Document Naming Rule', { refresh: function(frm) { frm.trigger('document_type'); - frm.trigger("add_button") + frm.trigger("add_button"); }, document_type: (frm) => { // update the select field options with fieldnames @@ -32,7 +32,7 @@ frappe.ui.form.on('Document Naming Rule', { default: frm.doc.counter, reqd: 1, description: __('This will update the counter and will affect all documents that will be created') - }] + }]; let primary_action_label = __('Save'); @@ -44,10 +44,10 @@ frappe.ui.form.on('Document Naming Rule', { new_counter: fields.new_counter }, callback: function() { - frm.set_value("counter", fields.new_counter) - dialog.hide() + frm.set_value("counter", fields.new_counter); + dialog.hide(); } - }) + }); }; var dialog = new frappe.ui.Dialog({ @@ -57,7 +57,7 @@ frappe.ui.form.on('Document Naming Rule', { primary_action }); - dialog.show() + dialog.show(); }); } From ed1a9e15591b7e8d4e1053fd97e528bbfb9cc43d Mon Sep 17 00:00:00 2001 From: hasnain2808 Date: Thu, 13 May 2021 09:55:27 +0530 Subject: [PATCH 07/49] fix: do not show counter on unsaved forms --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index e837ca2d6c..23a22ee33c 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -4,7 +4,7 @@ frappe.ui.form.on('Document Naming Rule', { refresh: function(frm) { frm.trigger('document_type'); - frm.trigger("add_button"); + if(!frm.doc.__islocal) frm.trigger("add_button"); }, document_type: (frm) => { // update the select field options with fieldnames From 90f786b0e5641f8c03b7e7e33988c82412aa4d4b Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Thu, 13 May 2021 09:57:10 +0530 Subject: [PATCH 08/49] chore: spaces --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 23a22ee33c..7dbf533975 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -4,7 +4,7 @@ frappe.ui.form.on('Document Naming Rule', { refresh: function(frm) { frm.trigger('document_type'); - if(!frm.doc.__islocal) frm.trigger("add_button"); + if (!frm.doc.__islocal) frm.trigger("add_button"); }, document_type: (frm) => { // update the select field options with fieldnames From 2e1c4650baf004c2a0e756d388bd4389c9335853 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Thu, 13 May 2021 19:54:56 +0530 Subject: [PATCH 09/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 7dbf533975..5e77e71820 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -51,7 +51,7 @@ frappe.ui.form.on('Document Naming Rule', { }; var dialog = new frappe.ui.Dialog({ - title: __('Update Counter Value for Prefix: ' + frm.doc.prefix), + title: __('Update Counter Value for Prefix: {0}', [frm.doc.prefix]), fields, primary_action_label, primary_action From c551338c0d7a665730ba845ccda5b4f09107301e Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 18 May 2021 15:41:31 +0530 Subject: [PATCH 10/49] fix: ensure website theme is applied correctly --- frappe/public/scss/website/css_variables.scss | 2 ++ frappe/public/scss/website/sidebar.scss | 6 +++--- .../doctype/website_theme/website_theme_template.scss | 11 +++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/frappe/public/scss/website/css_variables.scss b/frappe/public/scss/website/css_variables.scss index 2aeb842802..463a0db49e 100644 --- a/frappe/public/scss/website/css_variables.scss +++ b/frappe/public/scss/website/css_variables.scss @@ -28,4 +28,6 @@ --font-size-4xl: #{$font-size-4xl}; --font-size-5xl: #{$font-size-5xl}; --font-size-6xl: #{$font-size-6xl}; + + --card-border-radius: #{$card-border-radius}; } diff --git a/frappe/public/scss/website/sidebar.scss b/frappe/public/scss/website/sidebar.scss index b13eaf2a74..76956c9136 100644 --- a/frappe/public/scss/website/sidebar.scss +++ b/frappe/public/scss/website/sidebar.scss @@ -10,7 +10,7 @@ margin-top: 0.25rem; border-radius: 0.375rem; font-size: $font-size-sm; - color: $gray-600; + color: var(--text-color); text-decoration: none; font-weight: 500; @include transition(); @@ -26,8 +26,8 @@ } .sidebar-item a.active { - color: $primary; - background-color: $primary-light; + color: var(--primary); + background-color: var(--primary-light); } .sidebar-item-icon { diff --git a/frappe/website/doctype/website_theme/website_theme_template.scss b/frappe/website/doctype/website_theme/website_theme_template.scss index fbd640690b..2a67baf541 100644 --- a/frappe/website/doctype/website_theme/website_theme_template.scss +++ b/frappe/website/doctype/website_theme/website_theme_template.scss @@ -30,3 +30,14 @@ body { // Custom Theme {{ custom_scss or '' }} + +:root { + --primary: #{$primary}; + --primary-color: #{$primary}; + --primary-light: #{$primary-light}; + --light: #{$light}; + --bg-color: #{$body-bg}; + --text-color: #{$body-color}; + --text-light: #{$body-color}; +} + From a74352cfad16c3ba3650087c7403aaee7431bae1 Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 18 May 2021 16:19:25 +0530 Subject: [PATCH 11/49] fix: button colors --- frappe/public/scss/common/buttons.scss | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/public/scss/common/buttons.scss b/frappe/public/scss/common/buttons.scss index 591dc5bba6..de3a4cfc20 100644 --- a/frappe/public/scss/common/buttons.scss +++ b/frappe/public/scss/common/buttons.scss @@ -48,13 +48,13 @@ $active-border: darken($primary-light, 12.5%) ); - color: var(--blue-500); + color: var(--primary); &:hover, &:active { - color: var(--blue-500); + color: var(--primary); } &:focus { - box-shadow: 0 0 0 0.2rem var(--blue-50) + box-shadow: 0 0 0 0.2rem var(--primary-light); } } @@ -77,11 +77,11 @@ } .btn.btn-primary { - background-color: var(--primary-color); + background-color: var(--primary); color: var(--white); white-space: nowrap; --icon-stroke: currentColor; - --icon-fill-bg: var(--primary-color); + --icon-fill-bg: var(--primary); } .btn.btn-danger { From 2b8aaa53759f4f6168f6572dbda79092b2517437 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 18 May 2021 21:12:45 +0530 Subject: [PATCH 12/49] fix(minor): expose limited methods of json module --- frappe/utils/safe_exec.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 6c1fa21685..2472de70f0 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -61,7 +61,9 @@ def get_safe_globals(): out = NamespaceDict( # make available limited methods of frappe - json=json, + json=NamespaceDict( + loads = json.loads, + dumps = json.dumps), dict=dict, log=frappe.log, _dict=frappe._dict, From 98100437d0f3476c88814159929e3a73dbabea40 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 20 May 2021 16:52:48 +0530 Subject: [PATCH 13/49] chore: Renamed test files "appropriately" --- frappe/tests/{test_api.py => test_frappe_client.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename frappe/tests/{test_api.py => test_frappe_client.py} (100%) diff --git a/frappe/tests/test_api.py b/frappe/tests/test_frappe_client.py similarity index 100% rename from frappe/tests/test_api.py rename to frappe/tests/test_frappe_client.py From bcbc14d047cc16b3847d03bc7db4b28d0b2f363a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 20 May 2021 17:11:12 +0530 Subject: [PATCH 14/49] chore: Rename test suite and remove unused imports --- frappe/tests/test_frappe_client.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py index 6453062877..e1cdbb6ccd 100644 --- a/frappe/tests/test_frappe_client.py +++ b/frappe/tests/test_frappe_client.py @@ -1,8 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import unicode_literals -import unittest, frappe, os +import unittest, frappe from frappe.core.doctype.user.user import generate_keys from frappe.frappeclient import FrappeClient, FrappeException from frappe.utils.data import get_url @@ -10,7 +9,7 @@ from frappe.utils.data import get_url import requests import base64 -class TestAPI(unittest.TestCase): +class TestFrappeClient(unittest.TestCase): def test_insert_many(self): server = FrappeClient(get_url(), "Administrator", "admin", verify=False) frappe.db.sql("delete from `tabNote` where title in ('Sing','a','song','of','sixpence')") From 4a814571a8008672cbae058351a21d9a8665b714 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 20 May 2021 19:26:11 +0530 Subject: [PATCH 15/49] test: Added tests for Frappe APIs * Added tests for /api/resource usage * Added tests for /api/method base endpoints --- frappe/tests/test_api.py | 170 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 frappe/tests/test_api.py diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py new file mode 100644 index 0000000000..4d00df22a5 --- /dev/null +++ b/frappe/tests/test_api.py @@ -0,0 +1,170 @@ +import unittest +from random import choice + +import requests +from semantic_version import Version + +import frappe +from frappe.utils import get_site_url + + +def maintain_state(f): + def wrapper(*args, **kwargs): + frappe.db.rollback() + r = f(*args, **kwargs) + frappe.db.commit() + return r + + return wrapper + + +class TestResourceAPI(unittest.TestCase): + SITE_URL = get_site_url(frappe.local.site) + RESOURCE_URL = f"{SITE_URL}/api/resource" + DOCTYPE = "ToDo" + GENERATED_DOCUMENTS = [] + + @classmethod + @maintain_state + def setUpClass(self): + for _ in range(10): + doc = frappe.get_doc( + {"doctype": "ToDo", "description": frappe.mock("paragraph")} + ).insert() + self.GENERATED_DOCUMENTS.append(doc.name) + + @classmethod + @maintain_state + def tearDownClass(self): + for name in self.GENERATED_DOCUMENTS: + frappe.delete_doc_if_exists(self.DOCTYPE, name) + + @property + def sid(self): + if not getattr(self, "_sid", None): + self._sid = requests.post( + f"{self.SITE_URL}/api/method/login", + data={ + "usr": "Administrator", + "pwd": "root" or frappe.conf.admin_password or "admin", + }, + ).cookies.get("sid") + + return self._sid + + def get(self, path, params=""): + return requests.get(f"{self.RESOURCE_URL}/{path}?sid={self.sid}{params}") + + def post(self, path, data): + return requests.post( + f"{self.RESOURCE_URL}/{path}?sid={self.sid}", data=frappe.as_json(data) + ) + + def put(self, path, data): + return requests.put( + f"{self.RESOURCE_URL}/{path}?sid={self.sid}", data=frappe.as_json(data) + ) + + def delete(self, path): + return requests.delete(f"{self.RESOURCE_URL}/{path}?sid={self.sid}") + + def test_unauthorized_call(self): + # test 1: fetch documents without auth + response = requests.get(f"{self.RESOURCE_URL}/{self.DOCTYPE}") + self.assertEqual(response.status_code, 403) + + def test_get_list(self): + # test 2: fetch documents without params + response = self.get(self.DOCTYPE) + self.assertEqual(response.status_code, 200) + self.assertIsInstance(response.json(), dict) + self.assertIn("data", response.json()) + + def test_get_list_limit(self): + # test 3: fetch data with limit + response = self.get(self.DOCTYPE, "&limit=2") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json()["data"]), 2) + + def test_get_list_dict(self): + # test 4: fetch response as (not) dict + response = self.get(self.DOCTYPE, "&as_dict=True") + json = frappe._dict(response.json()) + self.assertEqual(response.status_code, 200) + self.assertIsInstance(json.data, list) + self.assertIsInstance(json.data[0], dict) + + response = self.get(self.DOCTYPE, "&as_dict=False") + json = frappe._dict(response.json()) + self.assertEqual(response.status_code, 200) + self.assertIsInstance(json.data, list) + self.assertIsInstance(json.data[0], list) + + def test_get_list_debug(self): + # test 5: fetch response with debug + response = self.get(self.DOCTYPE, "&debug=true") + self.assertEqual(response.status_code, 200) + self.assertIn("exc", response.json()) + self.assertIsInstance(response.json()["exc"], str) + self.assertIsInstance(eval(response.json()["exc"]), list) + + def test_get_list_fields(self): + # test 6: fetch response with fields + response = self.get(self.DOCTYPE, r'&fields=["description"]') + self.assertEqual(response.status_code, 200) + json = frappe._dict(response.json()) + self.assertIn("description", json.data[0]) + + def test_create_document(self): + # test 7: POST method on /api/resource to create doc + data = {"description": frappe.mock("paragraph")} + response = self.post(self.DOCTYPE, data) + self.assertEqual(response.status_code, 200) + docname = response.json()["data"]["name"] + self.assertIsInstance(docname, str) + self.GENERATED_DOCUMENTS.append(docname) + + def test_update_document(self): + # test 8: PUT method on /api/resource to update doc + generated_desc = frappe.mock("paragraph") + data = {"description": generated_desc} + random_doc = choice(self.GENERATED_DOCUMENTS) + desc_before_update = frappe.db.get_value(self.DOCTYPE, random_doc, "description") + + response = self.put(f"{self.DOCTYPE}/{random_doc}", data=data) + self.assertEqual(response.status_code, 200) + self.assertNotEqual(response.json()["data"]["description"], desc_before_update) + self.assertEqual(response.json()["data"]["description"], generated_desc) + + def test_delete_document(self): + # test 9: DELETE method on /api/resource + doc_to_delete = choice(self.GENERATED_DOCUMENTS) + response = self.delete(f"{self.DOCTYPE}/{doc_to_delete}") + self.assertEqual(response.status_code, 202) + self.assertDictEqual(response.json(), {"message": "ok"}) + + non_existent_doc = frappe.generate_hash(length=12) + response = self.delete(f"{self.DOCTYPE}/{non_existent_doc}") + self.assertEqual(response.status_code, 404) + self.assertDictEqual(response.json(), {}) + + +class TestMethodAPI(unittest.TestCase): + METHOD_URL = f"{get_site_url(frappe.local.site)}/api/method" + + def test_version(self): + # test 1: test for /api/method/version + response = requests.get(f"{self.METHOD_URL}/version") + json = frappe._dict(response.json()) + + self.assertEqual(response.status_code, 200) + self.assertIsInstance(json, dict) + self.assertIsInstance(json.message, str) + self.assertEqual(Version(json.message), Version(frappe.__version__)) + + def test_ping(self): + # test 2: test for /api/method/ping + response = requests.get(f"{self.METHOD_URL}/ping") + self.assertEqual(response.status_code, 200) + self.assertIsInstance(response.json(), dict) + self.assertEqual(response.json()['message'], "pong") From 969e2b6db95d1c865730776921e853cdeb0284ef Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Thu, 20 May 2021 20:04:40 +0530 Subject: [PATCH 16/49] fix: allow only system manager Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/core/doctype/document_naming_rule/document_naming_rule.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 13d54dffdd..7fa7c73efd 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -33,5 +33,6 @@ class DocumentNamingRule(Document): @frappe.whitelist() def update_current(name, new_counter): + frappe.only_for('System Manager') frappe.db.set_value('Document Naming Rule', name, 'counter', new_counter) frappe.db.commit() From 730bcc8c035bc5aec5173f1feb0b06a2e5c664de Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Thu, 20 May 2021 20:05:00 +0530 Subject: [PATCH 17/49] fix: do not explicitly commit Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/core/doctype/document_naming_rule/document_naming_rule.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 7fa7c73efd..653c056caa 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -35,4 +35,3 @@ class DocumentNamingRule(Document): def update_current(name, new_counter): frappe.only_for('System Manager') frappe.db.set_value('Document Naming Rule', name, 'counter', new_counter) - frappe.db.commit() From e16988b887d129c13fe30f9d67e27ff0fad93a82 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Thu, 20 May 2021 20:05:58 +0530 Subject: [PATCH 18/49] chore: change description --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 5e77e71820..f4b7428aff 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -31,7 +31,7 @@ frappe.ui.form.on('Document Naming Rule', { label: __('New Counter'), default: frm.doc.counter, reqd: 1, - description: __('This will update the counter and will affect all documents that will be created') + description: __('Updating counter may lead to document name conflicts if not done properly') }]; let primary_action_label = __('Save'); From dfc23eb3a3e42869430cf686a29f0b4188894618 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 21 May 2021 11:22:19 +0530 Subject: [PATCH 19/49] fix(test): Set admin password correctly --- frappe/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index 4d00df22a5..7e77aab779 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -46,7 +46,7 @@ class TestResourceAPI(unittest.TestCase): f"{self.SITE_URL}/api/method/login", data={ "usr": "Administrator", - "pwd": "root" or frappe.conf.admin_password or "admin", + "pwd": frappe.conf.admin_password or "admin", }, ).cookies.get("sid") From c9dc4ee441eae931b0d1918ba18fd0ce81df5967 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 21 May 2021 11:38:36 +0530 Subject: [PATCH 20/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index f4b7428aff..cd85615f5f 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -31,7 +31,7 @@ frappe.ui.form.on('Document Naming Rule', { label: __('New Counter'), default: frm.doc.counter, reqd: 1, - description: __('Updating counter may lead to document name conflicts if not done properly') + description: __('Warning: Updating counter may lead to document name conflicts if not done properly') }]; let primary_action_label = __('Save'); From bcb4c5182ffdb9d7d70b239d8ee6b222ece63f0e Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 21 May 2021 11:38:43 +0530 Subject: [PATCH 21/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index cd85615f5f..b1123cba28 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -50,7 +50,7 @@ frappe.ui.form.on('Document Naming Rule', { }); }; - var dialog = new frappe.ui.Dialog({ + const dialog = new frappe.ui.Dialog({ title: __('Update Counter Value for Prefix: {0}', [frm.doc.prefix]), fields, primary_action_label, From 3f056d147dea656dd531c7e794da05a513aa44f7 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 21 May 2021 11:38:49 +0530 Subject: [PATCH 22/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index b1123cba28..3aea9e04a7 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -4,7 +4,7 @@ frappe.ui.form.on('Document Naming Rule', { refresh: function(frm) { frm.trigger('document_type'); - if (!frm.doc.__islocal) frm.trigger("add_button"); + if (!frm.doc.__islocal) frm.trigger("add_update_counter_button"); }, document_type: (frm) => { // update the select field options with fieldnames From fa53c47e58cf7f8c85bd1ba13ad6973ec998b2d0 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 21 May 2021 11:38:57 +0530 Subject: [PATCH 23/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 3aea9e04a7..0f0bdd149c 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -22,7 +22,7 @@ frappe.ui.form.on('Document Naming Rule', { }); } }, - add_button: (frm) => { + add_update_counter_button: (frm) => { frm.add_custom_button(__('Update Counter'), function() { const fields = [{ From 58400d6214ec61ff9733a4caf46bad18dff71880 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Fri, 21 May 2021 14:15:14 +0530 Subject: [PATCH 24/49] Update frappe/core/doctype/document_naming_rule/document_naming_rule.js Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../core/doctype/document_naming_rule/document_naming_rule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.js b/frappe/core/doctype/document_naming_rule/document_naming_rule.js index 0f0bdd149c..097a4e9a6e 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.js +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.js @@ -44,7 +44,7 @@ frappe.ui.form.on('Document Naming Rule', { new_counter: fields.new_counter }, callback: function() { - frm.set_value("counter", fields.new_counter); + frm.set_value("counter", fields.new_counter); dialog.hide(); } }); From 5b96b79ed43a391ad82f43bda408aeb99596a8ae Mon Sep 17 00:00:00 2001 From: leela Date: Wed, 12 May 2021 13:49:43 +0530 Subject: [PATCH 25/49] refactor: incoming mails --- .../doctype/communication/communication.py | 40 +- .../doctype/email_account/email_account.py | 395 +++--------------- .../email/doctype/email_queue/email_queue.py | 5 + frappe/email/email_body.py | 4 +- frappe/email/receive.py | 368 +++++++++++++++- frappe/hooks.py | 6 +- 6 files changed, 461 insertions(+), 357 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 5ebf714645..a9c1c1f5a3 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -21,9 +21,11 @@ from frappe.automation.doctype.assignment_rule.assignment_rule import apply as a exclude_from_linked_with = True class Communication(Document): + """Communication represents an external communication like Email. + """ no_feed_on_delete = True + DOCTYPE = 'Communication' - """Communication represents an external communication like Email.""" def onload(self): """create email flag queue""" if self.communication_type == "Communication" and self.communication_medium == "Email" \ @@ -149,6 +151,23 @@ class Communication(Document): self.email_status = "Spam" + @classmethod + def find(cls, name, ignore_error=False): + try: + return frappe.get_doc(cls.DOCTYPE, name) + except frappe.DoesNotExistError: + if ignore_error: + return + raise + + @classmethod + def find_one_by_filters(cls, *, order_by=None, **kwargs): + name = frappe.db.get_value(cls.DOCTYPE, kwargs, order_by=order_by) + return cls.find(name) if name else None + + def update_db(self, **kwargs): + frappe.db.set_value(self.DOCTYPE, self.name, kwargs) + def set_sender_full_name(self): if not self.sender_full_name and self.sender: if self.sender == "Administrator": @@ -180,6 +199,8 @@ class Communication(Document): if not self.sender_full_name: self.sender_full_name = sender_email + + def send(self, print_html=None, print_format=None, attachments=None, send_me_a_copy=False, recipients=None): """Send communication via Email. @@ -306,6 +327,21 @@ class Communication(Document): if autosave: self.save(ignore_permissions=ignore_permissions) + # @classmethod + # def add_incoming_mail(cls, email, uid, seen_status): + # if exists_already(): + # pass # Modify UID + # else: + # pass # Call new() to create object +class InMailMixin: + @classmethod + def add_incoming_mail(cls, email, uid, seen_status): + if exists_already(): + pass # Modify UID + else: + pass # Call new() to create object + + def on_doctype_update(): """Add indexes in `tabCommunication`""" frappe.db.add_index("Communication", ["reference_doctype", "reference_name"]) @@ -485,4 +521,4 @@ def set_avg_response_time(parent, communication): response_times.append(response_time) if response_times: avg_response_time = sum(response_times) / len(response_times) - parent.db_set("avg_response_time", avg_response_time) \ No newline at end of file + parent.db_set("avg_response_time", avg_response_time) diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 36b662bb39..0a66efb480 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -19,7 +19,7 @@ from frappe.utils import (validate_email_address, cint, cstr, get_datetime, from frappe.utils.user import is_system_user from frappe.utils.jinja import render_template from frappe.email.smtp import SMTPServer -from frappe.email.receive import EmailServer, Email +from frappe.email.receive import EmailServer, Email, InboundMail, SentEmailInInboxError from poplib import error_proto from dateutil.relativedelta import relativedelta from datetime import datetime, timedelta @@ -430,89 +430,79 @@ class EmailAccount(Document): def receive(self, test_mails=None): """Called by scheduler to receive emails from this EMail account using POP3/IMAP.""" - def get_seen(status): - if not status: - return None - seen = 1 if status == "SEEN" else 0 - return seen + exceptions = [] + inbound_mails = self.get_inbound_mails(test_mails=test_mails) + for mail in inbound_mails: + try: + communication = mail.process() + frappe.db.commit() + # If email already exists in the system + # then do not send notifications for the same email. + if communication and mail.flags.is_new_communication: + # notify all participants of this thread + if self.enable_auto_reply: + self.send_auto_reply(communication, mail) - if self.enable_incoming: - uid_list = [] - exceptions = [] - seen_status = [] - uid_reindexed = False - email_server = None + attachments = [] + if hasattr(communication, '_attachments'): + attachments = [d.file_name for d in communication._attachments] + communication.notify(attachments=attachments, fetched_from_email_account=True) + except SentEmailInInboxError: + frappe.db.rollback() + except Exception: + frappe.db.rollback() + frappe.log_error('email_account.receive') + if self.use_imap: + self.handle_bad_emails(mail.uid, mail.raw_message, frappe.get_traceback()) + exceptions.append(frappe.get_traceback()) - if frappe.local.flags.in_test: - incoming_mails = test_mails or [] - else: - email_sync_rule = self.build_email_sync_rule() + #notify if user is linked to account + if len(inbound_mails)>0 and not frappe.local.flags.in_test: + frappe.publish_realtime('new_email', + {"account":self.email_account_name, "number":len(inbound_mails)} + ) - try: - email_server = self.get_incoming_server(in_receive=True, email_sync_rule=email_sync_rule) - except Exception: - frappe.log_error(title=_("Error while connecting to email account {0}").format(self.name)) + if exceptions: + raise Exception(frappe.as_json(exceptions)) - if not email_server: - return + def get_inbound_mails(self, test_mails=None): + """retrive and return inbound mails. - emails = email_server.get_messages() - if not emails: - return + TODO: Handle for tests + """ + if not self.enable_incoming: + return [] - incoming_mails = emails.get("latest_messages", []) - uid_list = emails.get("uid_list", []) - seen_status = emails.get("seen_status", []) - uid_reindexed = emails.get("uid_reindexed", False) + if frappe.local.flags.in_test: + return [InboundMail(msg, self) for msg in test_mails or []] - for idx, msg in enumerate(incoming_mails): - uid = None if not uid_list else uid_list[idx] - self.flags.notify = True + email_sync_rule = self.build_email_sync_rule() + try: + email_server = self.get_incoming_server(in_receive=True, email_sync_rule=email_sync_rule) + messages = email_server.get_messages() or {} + except Exception: + raise + print("In exception...") + frappe.log_error(title=_("Error while connecting to email account {0}").format(self.name)) + return [] - try: - args = { - "uid": uid, - "seen": None if not seen_status else get_seen(seen_status.get(uid, None)), - "uid_reindexed": uid_reindexed - } - communication = self.insert_communication(msg, args=args) + mails = [] + for index, message in enumerate(messages.get("latest_messages", [])): + print("RAW MESSAGE TYPE: ", type(message)) + uid = messages['uid_list'][index] + seen_status = 1 if messages['seen_status'][uid]=='SEEN' else 0 + mails.append(InboundMail(message, self, uid, seen_status)) - except SentEmailInInbox: - frappe.db.rollback() + return mails - except Exception: - frappe.db.rollback() - frappe.log_error('email_account.receive') - if self.use_imap: - self.handle_bad_emails(email_server, uid, msg, frappe.get_traceback()) - exceptions.append(frappe.get_traceback()) - - else: - frappe.db.commit() - if communication and self.flags.notify: - - # If email already exists in the system - # then do not send notifications for the same email. - - attachments = [] - - if hasattr(communication, '_attachments'): - attachments = [d.file_name for d in communication._attachments] - - communication.notify(attachments=attachments, fetched_from_email_account=True) - - #notify if user is linked to account - if len(incoming_mails)>0 and not frappe.local.flags.in_test: - frappe.publish_realtime('new_email', {"account":self.email_account_name, "number":len(incoming_mails)}) - - if exceptions: - raise Exception(frappe.as_json(exceptions)) - - def handle_bad_emails(self, email_server, uid, raw, reason): - if email_server and cint(email_server.settings.use_imap): + def handle_bad_emails(self, uid, raw, reason): + if cint(self.use_imap): import email try: - mail = email.message_from_string(raw) + if isinstance(raw, bytes): + mail = email.message_from_bytes(raw) + else: + mail = email.message_from_string(raw) message_id = mail.get('Message-ID') except Exception: @@ -524,275 +514,18 @@ class EmailAccount(Document): "reason":reason, "message_id": message_id, "doctype": "Unhandled Email", - "email_account": email_server.settings.email_account + "email_account": self.name }) unhandled_email.insert(ignore_permissions=True) frappe.db.commit() - def insert_communication(self, msg, args=None): - if isinstance(msg, list): - raw, uid, seen = msg - else: - raw = msg - uid = -1 - seen = 0 - if isinstance(args, dict): - if args.get("uid", -1): uid = args.get("uid", -1) - if args.get("seen", 0): seen = args.get("seen", 0) - - email = Email(raw) - - if email.from_email == self.email_id and not email.mail.get("Reply-To"): - # gmail shows sent emails in inbox - # and we don't want emails sent by us to be pulled back into the system again - # dont count emails sent by the system get those - if frappe.flags.in_test: - print('WARN: Cannot pull email. Sender sames as recipient inbox') - raise SentEmailInInbox - - if email.message_id: - # https://stackoverflow.com/a/18367248 - names = frappe.db.sql("""SELECT DISTINCT `name`, `creation` FROM `tabCommunication` - WHERE `message_id`='{message_id}' - ORDER BY `creation` DESC LIMIT 1""".format( - message_id=email.message_id - ), as_dict=True) - - if names: - name = names[0].get("name") - # email is already available update communication uid instead - frappe.db.set_value("Communication", name, "uid", uid, update_modified=False) - - self.flags.notify = False - - return frappe.get_doc("Communication", name) - - if email.content_type == 'text/html': - email.content = clean_email_html(email.content) - - communication = frappe.get_doc({ - "doctype": "Communication", - "subject": email.subject, - "content": email.content, - 'text_content': email.text_content, - "sent_or_received": "Received", - "sender_full_name": email.from_real_name, - "sender": email.from_email, - "recipients": email.mail.get("To"), - "cc": email.mail.get("CC"), - "email_account": self.name, - "communication_medium": "Email", - "uid": int(uid or -1), - "message_id": email.message_id, - "communication_date": email.date, - "has_attachment": 1 if email.attachments else 0, - "seen": seen or 0 - }) - - self.set_thread(communication, email) - if communication.seen: - # get email account user and set communication as seen - users = frappe.get_all("User Email", filters={ "email_account": self.name }, - fields=["parent"]) - users = list(set([ user.get("parent") for user in users ])) - communication._seen = json.dumps(users) - - communication.flags.in_receive = True - communication.insert(ignore_permissions=True) - - # save attachments - communication._attachments = email.save_attachments_in_doc(communication) - - # replace inline images - dirty = False - for file in communication._attachments: - if file.name in email.cid_map and email.cid_map[file.name]: - dirty = True - - email.content = email.content.replace("cid:{0}".format(email.cid_map[file.name]), - file.file_url) - - if dirty: - # not sure if using save() will trigger anything - communication.db_set("content", sanitize_html(email.content)) - - # notify all participants of this thread - if self.enable_auto_reply and getattr(communication, "is_first", False): - self.send_auto_reply(communication, email) - - return communication - - def set_thread(self, communication, email): - """Appends communication to parent based on thread ID. Will extract - parent communication and will link the communication to the reference of that - communication. Also set the status of parent transaction to Open or Replied. - - If no thread id is found and `append_to` is set for the email account, - it will create a new parent transaction (e.g. Issue)""" - parent = None - - parent = self.find_parent_from_in_reply_to(communication, email) - - if not parent and self.append_to: - self.set_sender_field_and_subject_field() - - if not parent and self.append_to: - parent = self.find_parent_based_on_subject_and_sender(communication, email) - - if not parent and self.append_to and self.append_to!="Communication": - parent = self.create_new_parent(communication, email) - - if parent: - communication.reference_doctype = parent.doctype - communication.reference_name = parent.name - - # check if message is notification and disable notifications for this message - isnotification = email.mail.get("isnotification") - if isnotification: - if "notification" in isnotification: - communication.unread_notification_sent = 1 - - def set_sender_field_and_subject_field(self): - '''Identify the sender and subject fields from the `append_to` DocType''' - # set subject_field and sender_field - meta = frappe.get_meta(self.append_to) - self.subject_field = None - self.sender_field = None - - if hasattr(meta, "subject_field"): - self.subject_field = meta.subject_field - - if hasattr(meta, "sender_field"): - self.sender_field = meta.sender_field - - def find_parent_based_on_subject_and_sender(self, communication, email): - '''Find parent document based on subject and sender match''' - parent = None - - if self.append_to and self.sender_field: - if self.subject_field: - if '#' in email.subject: - # try and match if ID is found - # document ID is appended to subject - # example "Re: Your email (#OPP-2020-2334343)" - parent_id = email.subject.rsplit('#', 1)[-1].strip(' ()') - if parent_id: - parent = frappe.db.get_all(self.append_to, filters = dict(name = parent_id), - fields = 'name') - - if not parent: - # try and match by subject and sender - # if sent by same sender with same subject, - # append it to old coversation - subject = frappe.as_unicode(strip(re.sub(r"(^\s*(fw|fwd|wg)[^:]*:|\s*(re|aw)[^:]*:\s*)*", - "", email.subject, 0, flags=re.IGNORECASE))) - - parent = frappe.db.get_all(self.append_to, filters={ - self.sender_field: email.from_email, - self.subject_field: ("like", "%{0}%".format(subject)), - "creation": (">", (get_datetime() - relativedelta(days=60)).strftime(DATE_FORMAT)) - }, fields = "name", limit = 1) - - if not parent and len(subject) > 10 and is_system_user(email.from_email): - # match only subject field - # when the from_email is of a user in the system - # and subject is atleast 10 chars long - parent = frappe.db.get_all(self.append_to, filters={ - self.subject_field: ("like", "%{0}%".format(subject)), - "creation": (">", (get_datetime() - relativedelta(days=60)).strftime(DATE_FORMAT)) - }, fields = "name", limit = 1) - - - - if parent: - parent = frappe._dict(doctype=self.append_to, name=parent[0].name) - return parent - - def create_new_parent(self, communication, email): - '''If no parent found, create a new reference document''' - - # no parent found, but must be tagged - # insert parent type doc - parent = frappe.new_doc(self.append_to) - - if self.subject_field: - parent.set(self.subject_field, frappe.as_unicode(email.subject)[:140]) - - if self.sender_field: - parent.set(self.sender_field, frappe.as_unicode(email.from_email)) - - if parent.meta.has_field("email_account"): - parent.email_account = self.name - - parent.flags.ignore_mandatory = True - - try: - parent.insert(ignore_permissions=True) - except frappe.DuplicateEntryError: - # try and find matching parent - parent_name = frappe.db.get_value(self.append_to, {self.sender_field: email.from_email}) - if parent_name: - parent.name = parent_name - else: - parent = None - - # NOTE if parent isn't found and there's no subject match, it is likely that it is a new conversation thread and hence is_first = True - communication.is_first = True - - return parent - - def find_parent_from_in_reply_to(self, communication, email): - '''Returns parent reference if embedded in In-Reply-To header - - Message-ID is formatted as `{message_id}@{site}`''' - parent = None - in_reply_to = (email.mail.get("In-Reply-To") or "").strip(" <>") - - if in_reply_to: - if "@{0}".format(frappe.local.site) in in_reply_to: - # reply to a communication sent from the system - email_queue = frappe.db.get_value('Email Queue', dict(message_id=in_reply_to), ['communication','reference_doctype', 'reference_name']) - if email_queue: - parent_communication, parent_doctype, parent_name = email_queue - if parent_communication: - communication.in_reply_to = parent_communication - else: - reference, domain = in_reply_to.split("@", 1) - parent_doctype, parent_name = 'Communication', reference - - if frappe.db.exists(parent_doctype, parent_name): - parent = frappe._dict(doctype=parent_doctype, name=parent_name) - - # set in_reply_to of current communication - if parent_doctype=='Communication': - # communication.in_reply_to = email_queue.communication - - if parent.reference_name: - # the true parent is the communication parent - parent = frappe.get_doc(parent.reference_doctype, - parent.reference_name) - else: - comm = frappe.db.get_value('Communication', - dict( - message_id=in_reply_to, - creation=['>=', add_days(get_datetime(), -30)]), - ['reference_doctype', 'reference_name'], as_dict=1) - if comm: - parent = frappe._dict(doctype=comm.reference_doctype, name=comm.reference_name) - - return parent - def send_auto_reply(self, communication, email): """Send auto reply if set.""" from frappe.core.doctype.communication.email import set_incoming_outgoing_accounts - if self.enable_auto_reply: set_incoming_outgoing_accounts(communication) - if self.send_unsubscribe_message: - unsubscribe_message = _("Leave this conversation") - else: - unsubscribe_message = "" + unsubscribe_message = (self.send_unsubscribe_message and _("Leave this conversation")) or "" frappe.sendmail(recipients = [email.from_email], sender = self.email_id, diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 076dfc5417..3c34182fe6 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -45,6 +45,11 @@ class EmailQueue(Document): def find(cls, name): return frappe.get_doc(cls.DOCTYPE, name) + @classmethod + def find_one_by_filters(cls, **kwargs): + name = frappe.db.get_value(cls.DOCTYPE, kwargs) + return cls.find(name) if name else None + def update_db(self, commit=False, **kwargs): frappe.db.set_value(self.DOCTYPE, self.name, kwargs) if commit: diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index 45888119ea..7a24583c83 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -359,9 +359,7 @@ def add_attachment(fname, fcontent, content_type=None, def get_message_id(): '''Returns Message ID created from doctype and name''' - return "<{unique}@{site}>".format( - site=frappe.local.site, - unique=email.utils.make_msgid(random_string(10)).split('@')[0].split('<')[1]) + return email.utils.make_msgid(domain=frappe.local.site) def get_signature(email_account): if email_account and email_account.add_signature and email_account.signature: diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 949da4a343..52b9110ddb 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -8,6 +8,7 @@ import imaplib import poplib import re import time +import json from email.header import decode_header import _socket @@ -20,13 +21,26 @@ from frappe import _, safe_decode, safe_encode from frappe.core.doctype.file.file import (MaxFileSizeReachedError, get_random_filename) from frappe.utils import (cint, convert_utc_to_user_timezone, cstr, - extract_email_id, markdown, now, parse_addr, strip) + extract_email_id, markdown, now, parse_addr, strip, get_datetime, + add_days, sanitize_html) +from frappe.utils.user import is_system_user +from frappe.utils.html_utils import clean_email_html + +# fix due to a python bug in poplib that limits it to 2048 +poplib._MAXLINE = 20480 +imaplib._MAXLINE = 20480 + +# fix due to a python bug in poplib that limits it to 2048 +poplib._MAXLINE = 20480 +imaplib._MAXLINE = 20480 class EmailSizeExceededError(frappe.ValidationError): pass class EmailTimeoutError(frappe.ValidationError): pass class TotalSizeExceededError(frappe.ValidationError): pass class LoginLimitExceeded(frappe.ValidationError): pass +class SentEmailInInboxError(Exception): + pass class EmailServer: """Wrapper for POP server to pull emails.""" @@ -100,14 +114,11 @@ class EmailServer: def get_messages(self): """Returns new email messages in a list.""" - if not self.check_mails(): - return # nothing to do + if not (self.check_mails() or self.connect()): + return [] frappe.db.commit() - if not self.connect(): - return - uid_list = [] try: @@ -116,7 +127,6 @@ class EmailServer: self.latest_messages = [] self.seen_status = {} self.uid_reindexed = False - uid_list = email_list = self.get_new_mails() if not email_list: @@ -132,11 +142,7 @@ class EmailServer: self.max_email_size = cint(frappe.local.conf.get("max_email_size")) self.max_total_size = 5 * self.max_email_size - for i, message_meta in enumerate(email_list): - # do not pull more than NUM emails - if (i+1) > num: - break - + for i, message_meta in enumerate(email_list[:num]): try: self.retrieve_message(message_meta, i+1) except (TotalSizeExceededError, EmailTimeoutError, LoginLimitExceeded): @@ -152,7 +158,6 @@ class EmailServer: except Exception as e: if self.has_login_limit_exceeded(e): pass - else: raise @@ -284,7 +289,7 @@ class EmailServer: flags = [] for flag in imaplib.ParseFlags(flag_string) or []: - pattern = re.compile("\w+") + pattern = re.compile(r"\w+") match = re.search(pattern, frappe.as_unicode(flag)) flags.append(match.group(0)) @@ -369,6 +374,7 @@ class Email: else: self.mail = email.message_from_string(content) + self.raw_message = content self.text_content = '' self.html_content = '' self.attachments = [] @@ -391,6 +397,10 @@ class Email: if self.date > now(): self.date = now() + @property + def in_reply_to(self): + return (self.mail.get("In-Reply-To") or "").strip(" <>") + def parse(self): """Walk and process multi-part email.""" for part in self.mail.walk(): @@ -555,13 +565,335 @@ class Email: def get_thread_id(self): """Extract thread ID from `[]`""" - l = re.findall('(?<=\[)[\w/-]+', self.subject) + l = re.findall(r'(?<=\[)[\w/-]+', self.subject) return l and l[0] or None + def is_reply(self): + return bool(self.in_reply_to) -# fix due to a python bug in poplib that limits it to 2048 -poplib._MAXLINE = 20480 -imaplib._MAXLINE = 20480 +class InboundMail(Email): + """Class representation of incoming mail along with mail handlers. + """ + def __init__(self, content, email_account, uid=None, seen_status=None): + super().__init__(content) + self.email_account = email_account + self.uid = uid or -1 + self.seen_status = seen_status or 0 + + # System documents related to this mail + self._parent_email_queue = None + self._parent_communication = None + self._reference_document = None + + self.flags = frappe._dict() + + def get_content(self): + if self.content_type == 'text/html': + return clean_email_html(self.content) + + def process(self): + """Create communication record from email. + """ + if self.is_sender_same_as_receiver() and not self.is_reply(): + if frappe.flags.in_test: + print('WARN: Cannot pull email. Sender same as recipient inbox') + raise SentEmailInInboxError + + communication = self.is_exist_in_system() + if communication: + communication.update_db(uid=self.uid) + communication.reload() + return communication + + self.flags.is_new_communication = True + return self._build_communication_doc() + + def _build_communication_doc(self): + data = self.as_dict() + data['doctype'] = "Communication" + + if self.parent_communication(): + data['in_reply_to'] = self.parent_communication().name + + if self.reference_document(): + data['reference_doctype'] = self.reference_document().doctype + data['reference_name'] = self.reference_document().name + elif self.email_account.append_to and self.email_account.append_to != 'Communication': + reference_doc = self._create_reference_document(self.email_account.append_to) + if reference_doc: + data['reference_doctype'] = reference_doc.doctype + data['reference_name'] = reference_doc.name + data['is_first'] = True + + if self.is_notification(): + # Disable notifications for notification. + data['unread_notification_sent'] = 1 + + if self.seen_status: + data['_seen'] = json.dumps(self.get_users_linked_to_account(self.email_account)) + + communication = frappe.get_doc(data) + communication.flags.in_receive = True + communication.insert(ignore_permissions=True) + + # save attachments + communication._attachments = self.save_attachments_in_doc(communication) + communication.content = sanitize_html(self.replace_inline_images(communication._attachments)) + communication.save() + return communication + + def replace_inline_images(self, attachments): + # replace inline images + content = self.content + for file in attachments: + if file.name in self.cid_map and self.cid_map[file.name]: + content = content.replace("cid:{0}".format(self.cid_map[file.name]), + file.file_url) + return content + + def is_notification(self): + isnotification = self.mail.get("isnotification") + return isnotification and ("notification" in isnotification) + + def is_exist_in_system(self): + """Check if this email already exists in the system(as communication document). + """ + from frappe.core.doctype.communication.communication import Communication + if not self.message_id: + return + + return Communication.find_one_by_filters(message_id = self.message_id, + order_by = 'creation DESC') + + def is_sender_same_as_receiver(self): + return self.from_email == self.email_account.email_id + + def is_reply_to_system_sent_mail(self): + """Is it a reply to already sent mail. + """ + return self.is_reply() and frappe.local.site in self.in_reply_to + + def parent_email_queue(self): + """Get parent record from `Email Queue`. + + If it is a reply to already sent mail, then there will be a parent record in EMail Queue. + """ + from frappe.email.doctype.email_queue.email_queue import EmailQueue + + if self._parent_email_queue is not None: + return self._parent_email_queue + + parent_email_queue = '' + if self.is_reply_to_system_sent_mail(): + parent_email_queue = EmailQueue.find_one_by_filters(message_id=self.in_reply_to) + + self._parent_email_queue = parent_email_queue or '' + return self._parent_email_queue + + def parent_communication(self): + """Find a related communication so that we can prepare a mail thread. + + The way it happens is by using in-reply-to header, and we can't make thread if it does not exist. + + Here are the cases to handle: + 1. If mail is a reply to already sent mail, then we can get parent communicaion from + Email Queue record. + 2. Sometimes we send communication name in message-ID directly, use that to get parent communication. + 3. Sender sent a reply but reply is on top of what (s)he sent before, + then parent record exists directly in communication. + """ + from frappe.core.doctype.communication.communication import Communication + if self._parent_communication is not None: + return self._parent_communication + + if not self.is_reply(): + return '' + + if not self.is_reply_to_system_sent_mail(): + communication = Communication.find_one_by_filters(message_id=self.in_reply_to, + creation = ['>=', self.get_relative_dt(-30)]) + elif self.parent_email_queue() and self.parent_email_queue().communication: + communication = Communication.find(self.parent_email_queue().communication, ignore_error=True) + else: + reference = self.in_reply_to + if '@' in self.in_reply_to: + reference, _ = self.in_reply_to.split("@", 1) + communication = Communication.find(reference, ignore_error=True) + + self._parent_communication = communication or '' + return self._parent_communication + + def reference_document(self): + """Reference document is a document to which mail relate to. + + We can get reference document from Parent record(EmailQueue | Communication) if exists. + Otherwise we do subject match to find reference document if we know the reference(append_to) doctype. + """ + if self._reference_document is not None: + return self._reference_document + + reference_document = "" + parent = self.parent_email_queue() or self.parent_communication() + + if parent and parent.reference_doctype: + reference_doctype, reference_name = parent.reference_doctype, parent.reference_name + reference_document = self.get_doc(reference_doctype, reference_name, ignore_error=True) + + if not reference_document and self.email_account.append_to: + reference_document = self.match_record_by_subject_and_sender(self.email_account.append_to) + + # if not reference_document: + # reference_document = Create_reference_document(self.email_account.append_to) + + self._reference_document = reference_document or '' + return self._reference_document + + def get_reference_name_from_subject(self): + """ + Ex: "Re: Your email (#OPP-2020-2334343)" + """ + return self.subject.rsplit('#', 1)[-1].strip(' ()') + + def match_record_by_subject_and_sender(self, doctype): + """Find a record in the given doctype that matches with email subject and sender. + + Cases: + 1. Sometimes record name is part of subject. We can get document by parsing name from subject + 2. Find by matching sender and subject + 3. Find by matching subject alone (Special case) + Ex: when a System User is using Outlook and replies to an email from their own client, + it reaches the Email Account with the threading info lost and the (sender + subject match) + doesn't work because the sender in the first communication was someone different to whom + the system user is replying to via the common email account in Frappe. This fix bypasses + the sender match when the sender is a system user and subject is atleast 10 chars long + (for additional safety) + + NOTE: We consider not to match by subject if match record is very old. + """ + name = self.get_reference_name_from_subject() + email_fields = self.get_email_fields(doctype) + + record = self.get_doc(doctype, name, ignore_error=True) if name else None + + if not record: + subject = self.clean_subject(self.subject) + filters = { + email_fields.subject_field: ("like", f"%{subject}%"), + "creation": (">", self.get_relative_dt(days=-60)) + } + + # Sender check is not needed incase mail is from system user. + if not (len(subject) > 10 and is_system_user(self.from_email)): + filters[email_fields.sender_field] = self.from_email + + name = frappe.db.get_value(self.email_account.append_to, filters = filters) + record = self.get_doc(doctype, name, ignore_error=True) if name else None + return record + + def _create_reference_document(self, doctype): + """ Create reference document if it does not exist in the system. + """ + parent = frappe.new_doc(doctype) + email_fileds = self.get_email_fields(doctype) + + if email_fileds.subject_field: + parent.set(email_fileds.subject_field, frappe.as_unicode(self.subject)[:140]) + + if email_fileds.sender_field: + parent.set(email_fileds.sender_field, frappe.as_unicode(self.from_email)) + + parent.flags.ignore_mandatory = True + + try: + parent.insert(ignore_permissions=True) + except frappe.DuplicateEntryError: + # try and find matching parent + parent_name = frappe.db.get_value(self.email_account.append_to, + {email_fileds.sender_field: email.from_email} + ) + if parent_name: + parent.name = parent_name + else: + parent = None + return parent + + + @staticmethod + def get_doc(doctype, docname, ignore_error=False): + try: + print(doctype, docname) + return frappe.get_doc(doctype, docname) + except frappe.DoesNotExistError: + if ignore_error: + return + raise + + @staticmethod + def get_relative_dt(days): + """Get relative to current datetime. Only relative days are supported. + """ + return add_days(get_datetime(), days) + + @staticmethod + def get_users_linked_to_account(email_account): + """Get list of users who linked to Email account. + """ + users = frappe.get_all("User Email", filters={"email_account": email_account.name}, + fields=["parent"]) + return list(set([user.get("parent") for user in users])) + + @staticmethod + def clean_subject(subject): + """Remove Prefixes like 'fw', FWD', 're' etc from subject. + """ + # Match strings like "fw:", "re :" etc. + regex = r"(^\s*(fw|fwd|wg)[^:]*:|\s*(re|aw)[^:]*:\s*)*" + return frappe.as_unicode(strip(re.sub(regex, "", subject, 0, flags=re.IGNORECASE))) + + @staticmethod + def get_email_fields(doctype): + """Returns Email related fields of a doctype. + """ + fields = frappe._dict() + + email_fields = ['subject_field', 'sender_field'] + meta = frappe.get_meta(doctype) + + for field in email_fields: + if hasattr(meta, field): + fields[field] = getattr(meta, field) + print("FIELDS::::", fields) + return fields + + @staticmethod + def get_document(self, doctype, name): + """Is same as frappe.get_doc but suppresses the DoesNotExist error. + """ + try: + return frappe.get_doc(doctype, name) + except frappe.DoesNotExistError: + return None + + def as_dict(self): + """ + """ + return { + "subject": self.subject, + "content": self.get_content(), + 'text_content': self.text_content, + "sent_or_received": "Received", + "sender_full_name": self.from_real_name, + "sender": self.from_email, + "recipients": self.mail.get("To"), + "cc": self.mail.get("CC"), + "email_account": self.email_account.name, + "communication_medium": "Email", + "uid": self.uid, + "message_id": self.message_id, + "communication_date": self.date, + "has_attachment": 1 if self.attachments else 0, + "seen": self.seen_status or 0 + } class TimerMixin(object): def __init__(self, *args, **kwargs): diff --git a/frappe/hooks.py b/frappe/hooks.py index 1c78d47755..740aa94d9f 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -202,9 +202,9 @@ scheduler_events = { ] }, "all": [ - "frappe.email.queue.flush", - "frappe.email.doctype.email_account.email_account.pull", - "frappe.email.doctype.email_account.email_account.notify_unreplied", + # "frappe.email.queue.flush", + # "frappe.email.doctype.email_account.email_account.pull", + # "frappe.email.doctype.email_account.email_account.notify_unreplied", "frappe.integrations.doctype.razorpay_settings.razorpay_settings.capture_payment", 'frappe.utils.global_search.sync_global_search', "frappe.monitor.flush", From 78468508d4258ec8f810fac9bc41551d1a498c56 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Sun, 23 May 2021 13:11:02 +0530 Subject: [PATCH 26/49] fix: grid navigation with keyboard on multiple pages --- frappe/public/js/frappe/form/grid_row.js | 41 ++++++++++++++++++------ 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index 453b8b5f24..c25d074416 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -523,7 +523,7 @@ export default class GridRow { // hide other var open_row = this.get_open_form(); - if (show===undefined) show = !!!open_row; + if (show === undefined) show = !open_row; // call blur document.activeElement && document.activeElement.blur(); @@ -588,19 +588,42 @@ export default class GridRow { this.wrapper.removeClass("grid-row-open"); } open_prev() { - const row_index = this.wrapper.index(); - if (this.grid.grid_rows[row_index - 1]) { - this.grid.grid_rows[row_index - 1].toggle_view(true); - } + if (!this.doc) return; + this.open_row_at_index(this.doc.idx - 2); } open_next() { - const row_index = this.wrapper.index(); - if (this.grid.grid_rows[row_index + 1]) { - this.grid.grid_rows[row_index + 1].toggle_view(true); - } else { + if (!this.doc) return; + + if (!this.open_row_at_index(this.doc.idx)) { this.grid.add_new_row(null, null, true); } } + open_row_at_index(row_index) { + if (!this.grid.data[row_index]) return; + + this.change_page_if_reqd(row_index); + this.grid.grid_rows[row_index].toggle_view(true); + return true; + } + change_page_if_reqd(row_index) { + const { + page_index, + page_length + } = this.grid.grid_pagination; + + row_index++; + let new_page; + + if (row_index <= (page_index - 1) * page_length) { + new_page = page_index - 1; + } else if (row_index > page_index * page_length) { + new_page = page_index + 1; + } + + if (new_page) { + this.grid.grid_pagination.go_to_page(new_page); + } + } refresh_field(fieldname, txt) { let df = this.docfields.find(col => { return col.fieldname === fieldname; From 39e6c040de40564b8186827a437b37f3ace54c00 Mon Sep 17 00:00:00 2001 From: leela Date: Fri, 21 May 2021 18:57:40 +0530 Subject: [PATCH 27/49] test: Incoming mail tests are added --- .../doctype/communication/communication.py | 17 -- .../doctype/email_account/email_account.py | 3 - .../email_account/test_email_account.py | 245 +++++++++++++++++- .../test_mails/incoming-self-sent.raw | 91 +++++++ .../incoming-subject-placeholder.raw | 183 +++++++++++++ frappe/hooks.py | 6 +- 6 files changed, 509 insertions(+), 36 deletions(-) create mode 100644 frappe/email/doctype/email_account/test_mails/incoming-self-sent.raw create mode 100644 frappe/email/doctype/email_account/test_mails/incoming-subject-placeholder.raw diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index a9c1c1f5a3..9879807033 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -199,8 +199,6 @@ class Communication(Document): if not self.sender_full_name: self.sender_full_name = sender_email - - def send(self, print_html=None, print_format=None, attachments=None, send_me_a_copy=False, recipients=None): """Send communication via Email. @@ -327,21 +325,6 @@ class Communication(Document): if autosave: self.save(ignore_permissions=ignore_permissions) - # @classmethod - # def add_incoming_mail(cls, email, uid, seen_status): - # if exists_already(): - # pass # Modify UID - # else: - # pass # Call new() to create object -class InMailMixin: - @classmethod - def add_incoming_mail(cls, email, uid, seen_status): - if exists_already(): - pass # Modify UID - else: - pass # Call new() to create object - - def on_doctype_update(): """Add indexes in `tabCommunication`""" frappe.db.add_index("Communication", ["reference_doctype", "reference_name"]) diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 0a66efb480..eea2dfc73e 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -468,7 +468,6 @@ class EmailAccount(Document): def get_inbound_mails(self, test_mails=None): """retrive and return inbound mails. - TODO: Handle for tests """ if not self.enable_incoming: return [] @@ -482,13 +481,11 @@ class EmailAccount(Document): messages = email_server.get_messages() or {} except Exception: raise - print("In exception...") frappe.log_error(title=_("Error while connecting to email account {0}").format(self.name)) return [] mails = [] for index, message in enumerate(messages.get("latest_messages", [])): - print("RAW MESSAGE TYPE: ", type(message)) uid = messages['uid_list'][index] seen_status = 1 if messages['seen_status'][uid]=='SEEN' else 0 mails.append(InboundMail(message, self, uid, seen_status)) diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index f87ee32bb1..7bf49593f4 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -2,6 +2,10 @@ # See license.txt from __future__ import unicode_literals +from frappe.core.doctype import communication +from frappe.core.doctype.communication.communication import Communication +from frappe.email.receive import InboundMail, SentEmailInInboxError, Email +from frappe.email.email_body import get_message_id import frappe, os import unittest, email @@ -16,30 +20,37 @@ from frappe.email.doctype.email_account.email_account import notify_unreplied from datetime import datetime, timedelta class TestEmailAccount(unittest.TestCase): + @classmethod + def setUpClass(cls): + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.db_set("enable_incoming", 1) + email_account.db_set("enable_auto_reply", 1) + + @classmethod + def tearDownClass(cls): + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.db_set("enable_incoming", 0) + def setUp(self): frappe.flags.mute_emails = False frappe.flags.sent_mail = None - - email_account = frappe.get_doc("Email Account", "_Test Email Account 1") - email_account.db_set("enable_incoming", 1) frappe.db.sql('delete from `tabEmail Queue`') + frappe.db.sql('delete from `tabUnhandled Email`') - def tearDown(self): - email_account = frappe.get_doc("Email Account", "_Test Email Account 1") - email_account.db_set("enable_incoming", 0) + def get_test_mail(self, fname): + with open(os.path.join(os.path.dirname(__file__), "test_mails", fname), "r") as f: + return f.read() def test_incoming(self): cleanup("test_sender@example.com") - with open(os.path.join(os.path.dirname(__file__), "test_mails", "incoming-1.raw"), "r") as f: - test_mails = [f.read()] + test_mails = [self.get_test_mail('incoming-1.raw')] email_account = frappe.get_doc("Email Account", "_Test Email Account 1") email_account.receive(test_mails=test_mails) comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"}) self.assertTrue("test_receiver@example.com" in comm.recipients) - # check if todo is created self.assertTrue(frappe.db.get_value(comm.reference_doctype, comm.reference_name, "name")) @@ -88,7 +99,7 @@ class TestEmailAccount(unittest.TestCase): email_account.receive(test_mails=test_mails) comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"}) - self.assertTrue("From: \"Microsoft Outlook\" <test_sender@example.com>" in comm.content) + self.assertTrue("From: "Microsoft Outlook" <test_sender@example.com>" in comm.content) self.assertTrue("This is an e-mail message sent automatically by Microsoft Outlook while" in comm.content) def test_incoming_attached_email_from_outlook_layers(self): @@ -101,7 +112,7 @@ class TestEmailAccount(unittest.TestCase): email_account.receive(test_mails=test_mails) comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"}) - self.assertTrue("From: \"Microsoft Outlook\" <test_sender@example.com>" in comm.content) + self.assertTrue("From: "Microsoft Outlook" <test_sender@example.com>" in comm.content) self.assertTrue("This is an e-mail message sent automatically by Microsoft Outlook while" in comm.content) def test_outgoing(self): @@ -166,7 +177,6 @@ class TestEmailAccount(unittest.TestCase): comm_list = frappe.get_all("Communication", filters={"sender":"test_sender@example.com"}, fields=["name", "reference_doctype", "reference_name"]) - # both communications attached to the same reference self.assertEqual(comm_list[0].reference_doctype, comm_list[1].reference_doctype) self.assertEqual(comm_list[0].reference_name, comm_list[1].reference_name) @@ -199,6 +209,215 @@ class TestEmailAccount(unittest.TestCase): self.assertEqual(comm_list[0].reference_doctype, event.doctype) self.assertEqual(comm_list[0].reference_name, event.name) + def test_auto_reply(self): + cleanup("test_sender@example.com") + + test_mails = [self.get_test_mail('incoming-1.raw')] + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.receive(test_mails=test_mails) + + comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"}) + self.assertTrue(frappe.db.get_value("Email Queue", {"reference_doctype": comm.reference_doctype, + "reference_name": comm.reference_name})) + + def test_handle_bad_emails(self): + mail_content = self.get_test_mail(fname="incoming-1.raw") + message_id = Email(mail_content).mail.get('Message-ID') + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.handle_bad_emails(uid=-1, raw=mail_content, reason="Testing") + self.assertTrue(frappe.db.get_value("Unhandled Email", {'message_id': message_id})) + +class TestInboundMail(unittest.TestCase): + @classmethod + def setUpClass(cls): + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.db_set("enable_incoming", 1) + + @classmethod + def tearDownClass(cls): + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + email_account.db_set("enable_incoming", 0) + + def tearDown(self): + cleanup() + frappe.db.sql('delete from `tabEmail Queue`') + frappe.db.sql('delete from `tabToDo`') + + def get_test_mail(self, fname): + with open(os.path.join(os.path.dirname(__file__), "test_mails", fname), "r") as f: + return f.read() + + def new_doc(self, doctype, **data): + doc = frappe.new_doc(doctype) + for field, value in data.items(): + setattr(doc, field, value) + doc.insert() + return doc + + def new_communication(self, **kwargs): + defaults = { + 'subject': "Test Subject" + } + d = {**defaults, **kwargs} + return self.new_doc('Communication', **d) + + def new_email_queue(self, **kwargs): + defaults = { + 'message_id': get_message_id().strip(" <>") + } + d = {**defaults, **kwargs} + return self.new_doc('Email Queue', **d) + + def new_todo(self, **kwargs): + defaults = { + 'description': "Description" + } + d = {**defaults, **kwargs} + return self.new_doc('ToDo', **d) + + def test_self_sent_mail(self): + """Check that we raise SentEmailInInboxError if the inbound mail is self sent mail. + """ + mail_content = self.get_test_mail(fname="incoming-self-sent.raw") + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 1, 1) + with self.assertRaises(SentEmailInInboxError): + inbound_mail.process() + + def test_mail_exist_validation(self): + """Do not create communication record if the mail is already downloaded into the system. + """ + mail_content = self.get_test_mail(fname="incoming-1.raw") + message_id = Email(mail_content).message_id + # Create new communication record in DB + communication = self.new_communication(message_id=message_id) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + new_communiction = inbound_mail.process() + + # Make sure that uid is changed to new uid + self.assertEqual(new_communiction.uid, 12345) + self.assertEqual(communication.name, new_communiction.name) + + def test_find_parent_email_queue(self): + """If the mail is reply to the already sent mail, there will be a email queue record. + """ + # Create email queue record + queue_record = self.new_email_queue() + + mail_content = self.get_test_mail(fname="reply-4.raw").replace( + "{{ message_id }}", queue_record.message_id + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + parent_queue = inbound_mail.parent_email_queue() + self.assertEqual(queue_record.name, parent_queue.name) + + def test_find_parent_communication_through_queue(self): + """Find parent communication of an inbound mail. + Cases where parent communication does exist: + 1. No parent communication is the mail is not a reply. + + Cases where parent communication does not exist: + 2. If mail is not a reply to system sent mail, then there can exist co + """ + # Create email queue record + communication = self.new_communication() + queue_record = self.new_email_queue(communication=communication.name) + mail_content = self.get_test_mail(fname="reply-4.raw").replace( + "{{ message_id }}", queue_record.message_id + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + parent_communication = inbound_mail.parent_communication() + self.assertEqual(parent_communication.name, communication.name) + + def test_find_parent_communication_for_self_reply(self): + """If the inbound email is a reply but not reply to system sent mail. + + Ex: User replied to his/her mail. + """ + message_id = "new-message-id" + mail_content = self.get_test_mail(fname="reply-4.raw").replace( + "{{ message_id }}", message_id + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + parent_communication = inbound_mail.parent_communication() + self.assertFalse(parent_communication) + + communication = self.new_communication(message_id=message_id) + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + parent_communication = inbound_mail.parent_communication() + self.assertEqual(parent_communication.name, communication.name) + + def test_find_parent_communication_from_header(self): + """Incase of header contains parent communication name + """ + communication = self.new_communication() + mail_content = self.get_test_mail(fname="reply-4.raw").replace( + "{{ message_id }}", f"<{communication.name}@{frappe.local.site}>" + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + parent_communication = inbound_mail.parent_communication() + self.assertEqual(parent_communication.name, communication.name) + + def test_reference_document(self): + # Create email queue record + todo = self.new_todo() + # communication = self.new_communication(reference_doctype='ToDo', reference_name=todo.name) + queue_record = self.new_email_queue(reference_doctype='ToDo', reference_name=todo.name) + mail_content = self.get_test_mail(fname="reply-4.raw").replace( + "{{ message_id }}", queue_record.message_id + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + reference_doc = inbound_mail.reference_document() + self.assertEqual(todo.name, reference_doc.name) + + def test_reference_document_by_record_name_in_subject(self): + # Create email queue record + todo = self.new_todo() + + mail_content = self.get_test_mail(fname="incoming-subject-placeholder.raw").replace( + "{{ subject }}", f"RE: (#{todo.name})" + ) + + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + reference_doc = inbound_mail.reference_document() + self.assertEqual(todo.name, reference_doc.name) + + def test_reference_document_by_subject_match(self): + subject = "New todo" + todo = self.new_todo(sender='test_sender@example.com', description=subject) + + mail_content = self.get_test_mail(fname="incoming-subject-placeholder.raw").replace( + "{{ subject }}", f"RE: {subject}" + ) + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + reference_doc = inbound_mail.reference_document() + self.assertEqual(todo.name, reference_doc.name) + + def test_create_communication_from_mail(self): + # Create email queue record + mail_content = self.get_test_mail(fname="incoming-2.raw") + email_account = frappe.get_doc("Email Account", "_Test Email Account 1") + inbound_mail = InboundMail(mail_content, email_account, 12345, 1) + communication = inbound_mail.process() + self.assertTrue(communication.is_first) + self.assertTrue(communication._attachments) + def cleanup(sender=None): filters = {} if sender: @@ -207,4 +426,4 @@ def cleanup(sender=None): names = frappe.get_list("Communication", filters=filters, fields=["name"]) for name in names: frappe.delete_doc_if_exists("Communication", name.name) - frappe.delete_doc_if_exists("Communication Link", {"parent": name.name}) \ No newline at end of file + frappe.delete_doc_if_exists("Communication Link", {"parent": name.name}) diff --git a/frappe/email/doctype/email_account/test_mails/incoming-self-sent.raw b/frappe/email/doctype/email_account/test_mails/incoming-self-sent.raw new file mode 100644 index 0000000000..a16eecccd5 --- /dev/null +++ b/frappe/email/doctype/email_account/test_mails/incoming-self-sent.raw @@ -0,0 +1,91 @@ +Delivered-To: test_receiver@example.com +Received: by 10.96.153.227 with SMTP id vj3csp416144qdb; + Mon, 15 Sep 2014 03:35:07 -0700 (PDT) +X-Received: by 10.66.119.103 with SMTP id kt7mr36981968pab.95.1410777306321; + Mon, 15 Sep 2014 03:35:06 -0700 (PDT) +Return-Path: +Received: from mail-pa0-x230.google.com (mail-pa0-x230.google.com [2607:f8b0:400e:c03::230]) + by mx.google.com with ESMTPS id dg10si22178346pdb.115.2014.09.15.03.35.06 + for + (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); + Mon, 15 Sep 2014 03:35:06 -0700 (PDT) +Received-SPF: pass (google.com: domain of test@example.com designates 2607:f8b0:400e:c03::230 as permitted sender) client-ip=2607:f8b0:400e:c03::230; +Authentication-Results: mx.google.com; + spf=pass (google.com: domain of test@example.com designates 2607:f8b0:400e:c03::230 as permitted sender) smtp.mail=test@example.com; + dkim=pass header.i=@gmail.com; + dmarc=pass (p=NONE dis=NONE) header.from=gmail.com +Received: by mail-pa0-f48.google.com with SMTP id hz1so6118714pad.21 + for ; Mon, 15 Sep 2014 03:35:06 -0700 (PDT) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=gmail.com; s=20120113; + h=from:content-type:subject:message-id:date:to:mime-version; + bh=rwiLijtF3lfy9M6cP/7dv2Hm7NJuBwFZn1OFsN8Tlvs=; + b=x7U4Ny3Kz2ULRJ7a04NDBrBTVhP2ImIB9n3LVNGQDnDonPUM5Ro/wZcxPTVnBWZ2L1 + o1bGfP+lhBrvYUlHsd5r4FYC0Uvpad6hbzLr0DGUQgPTxW4cGKbtDEAq+BR2JWd9f803 + vdjSWdGk8w2dt2qbngTqIZkm5U2XWjICDOAYuPIseLUgCFwi9lLyOSARFB7mjAa2YL7Q + Nswk7mbWU1hbnHP6jaBb0m8QanTc7Up944HpNDRxIrB1ZHgKzYhXtx8nhnOx588ZGIAe + E6tyG8IwogR11vLkkrBhtMaOme9PohYx4F1CSTiwspmDCadEzJFGRe//lEXKmZHAYH6g + 90Zg== +X-Received: by 10.70.38.135 with SMTP id g7mr22078275pdk.100.1410777305744; + Mon, 15 Sep 2014 03:35:05 -0700 (PDT) +Return-Path: +Received: from [192.168.0.100] ([27.106.4.70]) + by mx.google.com with ESMTPSA id zr6sm11025126pbc.50.2014.09.15.03.35.02 + for + (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); + Mon, 15 Sep 2014 03:35:04 -0700 (PDT) +From: Rushabh Mehta +Content-Type: multipart/alternative; boundary="Apple-Mail=_57F71261-5C3A-43F6-918B-4438B96F61AA" +Subject: test mail 🦄🌈😎 +Message-Id: <9143999C-8456-4399-9CF1-4A2DA9DD7711@gmail.com> +Date: Mon, 15 Sep 2014 16:04:57 +0530 +To: Rushabh Mehta +Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) +X-Mailer: Apple Mail (2.1878.6) + + +--Apple-Mail=_57F71261-5C3A-43F6-918B-4438B96F61AA +Content-Transfer-Encoding: 7bit +Content-Type: text/plain; + charset=us-ascii + +test mail + + + +@rushabh_mehta +https://erpnext.org + + +--Apple-Mail=_57F71261-5C3A-43F6-918B-4438B96F61AA +Content-Transfer-Encoding: quoted-printable +Content-Type: text/html; + charset=us-ascii + +test = +mail
+



@rushabh_mehta
+
+
= + +--Apple-Mail=_57F71261-5C3A-43F6-918B-4438B96F61AA-- diff --git a/frappe/email/doctype/email_account/test_mails/incoming-subject-placeholder.raw b/frappe/email/doctype/email_account/test_mails/incoming-subject-placeholder.raw new file mode 100644 index 0000000000..35ddf06b01 --- /dev/null +++ b/frappe/email/doctype/email_account/test_mails/incoming-subject-placeholder.raw @@ -0,0 +1,183 @@ +Return-path: +Envelope-to: test_receiver@example.com +Delivery-date: Wed, 27 Jan 2016 16:24:20 +0800 +Received: from 23-59-23-10.perm.iinet.net.au ([23.59.23.10]:62191 helo=DESKTOP7C66I2M) + by webcloud85.au.syrahost.com with esmtp (Exim 4.86) + (envelope-from ) + id 1aOLOj-002xFL-CP + for test_receiver@example.com; Wed, 27 Jan 2016 16:24:20 +0800 +From: +To: +References: +In-Reply-To: +Subject: RE: {{ subject }} +Date: Wed, 27 Jan 2016 16:24:09 +0800 +Message-ID: <000001d158dc$1b8363a0$528a2ae0$@example.com> +MIME-Version: 1.0 +Content-Type: multipart/mixed; + boundary="----=_NextPart_000_0001_01D1591F.29A7DC20" +X-Mailer: Microsoft Outlook 14.0 +Thread-Index: AQJZfZxrgcB9KnMqoZ+S4Qq9hcoSeZ3+vGiQ +Content-Language: en-au + +This is a multipart message in MIME format. + +------=_NextPart_000_0001_01D1591F.29A7DC20 +Content-Type: multipart/alternative; + boundary="----=_NextPart_001_0002_01D1591F.29A7DC20" + + +------=_NextPart_001_0002_01D1591F.29A7DC20 +Content-Type: text/plain; + charset="utf-8" +Content-Transfer-Encoding: quoted-printable + +Test purely for testing with the debugger has email attached + +=20 + +From: Notification [mailto:test_receiver@example.com]=20 +Sent: Wednesday, 27 January 2016 9:30 AM +To: test_receiver@example.com +Subject: Sales Invoice: SINV-12276 + +=20 + +test no 6 sent from bench to outlook to be replied to with messaging + + + + +------=_NextPart_001_0002_01D1591F.29A7DC20 +Content-Type: text/html; + charset="utf-8" +Content-Transfer-Encoding: quoted-printable + +hi there

Test purely for testing with the debugger has email = +attached

 

From:= + = +Notification [mailto:test_receiver@example.com]
Sent: Wednesday, 27 = +January 2016 9:30 AM
To: = +test_receiver@example.com
Subject: Sales Invoice: = +SINV-12276

 

test no 3 sent from bench to outlook to be replied to with = +messaging

fizz buzz

This email was sent to test_receiver@example.= +com and copied to SuperUser

Leave this conversation = +

hi

+------=_NextPart_001_0002_01D1591F.29A7DC20-- + +------=_NextPart_000_0001_01D1591F.29A7DC20 +Content-Type: message/rfc822 +Content-Transfer-Encoding: 7bit +Content-Disposition: attachment + +Received: from 203-59-223-10.perm.iinet.net.au ([23.59.23.10]:49772 helo=DESKTOP7C66I2M) + by webcloud85.au.syrahost.com with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) + (Exim 4.86) + (envelope-from ) + id 1aOEtO-003tI4-Kv + for test_receiver@example.com; Wed, 27 Jan 2016 09:27:30 +0800 +Return-Path: +From: "Microsoft Outlook" +To: +Subject: Microsoft Outlook Test Message +MIME-Version: 1.0 +Content-Type: text/plain; + charset="utf-8" +Content-Transfer-Encoding: quoted-printable +X-Mailer: Microsoft Outlook 14.0 +Thread-Index: AdFYoeN8x8wUI/+QSoCJkp33NKPVmw== + +This is an e-mail message sent automatically by Microsoft Outlook while = +testing the settings for your account. diff --git a/frappe/hooks.py b/frappe/hooks.py index 740aa94d9f..1c78d47755 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -202,9 +202,9 @@ scheduler_events = { ] }, "all": [ - # "frappe.email.queue.flush", - # "frappe.email.doctype.email_account.email_account.pull", - # "frappe.email.doctype.email_account.email_account.notify_unreplied", + "frappe.email.queue.flush", + "frappe.email.doctype.email_account.email_account.pull", + "frappe.email.doctype.email_account.email_account.notify_unreplied", "frappe.integrations.doctype.razorpay_settings.razorpay_settings.capture_payment", 'frappe.utils.global_search.sync_global_search', "frappe.monitor.flush", From 2bbf71ade0c12ac931a898783a6e8fc330738083 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 25 May 2021 15:30:12 +0530 Subject: [PATCH 28/49] fix: refactor code --- frappe/public/js/frappe/form/grid.js | 2 +- frappe/public/js/frappe/form/grid_row.js | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index 00f9d6abca..aa3cf51314 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -348,7 +348,7 @@ export default class Grid { d.idx = ri + 1; } if (d.name === undefined) { - d.name = "row" + d.idx; + d.name = "row " + d.idx; } if (this.grid_rows[ri] && !append_row) { var grid_row = this.grid_rows[ri]; diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index bbcf0f707b..f6da88df57 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -93,11 +93,8 @@ export default class GridRow { } else { data = this.grid.df.data; } - let index = -1; - - if (me.doc.name) { - index = data.findIndex(d => d.name === me.doc.name); - } + + const index = data.findIndex(d => d.name === me.doc.name); if (index > -1) { // mutate array directly, From d30e6f6a43b371aadb9adddf019ba08549c1ec8a Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 25 May 2021 15:34:22 +0530 Subject: [PATCH 29/49] fix: refactor code --- frappe/public/js/frappe/form/grid.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/form/grid.js b/frappe/public/js/frappe/form/grid.js index aa3cf51314..13b5ebe842 100644 --- a/frappe/public/js/frappe/form/grid.js +++ b/frappe/public/js/frappe/form/grid.js @@ -196,9 +196,7 @@ export default class Grid { tasks.push(() => { if (dirty) { this.refresh(); - if (this.frm) { - this.frm.script_manager.trigger(this.df.fieldname + "_delete", this.doctype); - } + this.frm && this.frm.script_manager.trigger(this.df.fieldname + "_delete", this.doctype); } }); From e79ce22d4d7070c814da333e166830f64ca16c07 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Tue, 25 May 2021 19:08:19 +0530 Subject: [PATCH 30/49] fix: double checkboxes showing in editable grid --- frappe/public/js/frappe/form/controls/check.js | 4 +++- frappe/public/scss/common/grid.scss | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/check.js b/frappe/public/js/frappe/form/controls/check.js index 658ef640a9..1209f52e6d 100644 --- a/frappe/public/js/frappe/form/controls/check.js +++ b/frappe/public/js/frappe/form/controls/check.js @@ -14,8 +14,10 @@ frappe.ui.form.ControlCheck = class ControlCheck extends frappe.ui.form.ControlD `).appendTo(this.parent); } set_input_areas() { - this.label_area = this.label_span = this.$wrapper.find(".label-area").get(0); this.input_area = this.$wrapper.find(".input-area").get(0); + if (this.only_input) return; + + this.label_area = this.label_span = this.$wrapper.find(".label-area").get(0); this.disp_area = this.$wrapper.find(".disp-area").get(0); } make_input() { diff --git a/frappe/public/scss/common/grid.scss b/frappe/public/scss/common/grid.scss index 3cc5139d9e..aac949b1bf 100644 --- a/frappe/public/scss/common/grid.scss +++ b/frappe/public/scss/common/grid.scss @@ -140,7 +140,7 @@ .checkbox { margin: 0px; text-align: center; - margin-top: 9px; + margin-top: var(--padding-sm); } textarea { From 45b709906fc78a50d670d3b5a7901e7b72d6d22a Mon Sep 17 00:00:00 2001 From: leela Date: Tue, 25 May 2021 19:03:30 +0530 Subject: [PATCH 31/49] fix: display currency in auto email report Currently currency is always showed as INR(default) irrespective of what currency field option is. To get the field currency, the whole document(record) is needed. Fixed it by passing the document record. (cherry picked from commit 2e58d669e22e7fc8804ddfd759cdfe24f03d2916) --- .../email/doctype/auto_email_report/auto_email_report.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/email/doctype/auto_email_report/auto_email_report.py b/frappe/email/doctype/auto_email_report/auto_email_report.py index 6f1cd8eebd..91ca518e67 100644 --- a/frappe/email/doctype/auto_email_report/auto_email_report.py +++ b/frappe/email/doctype/auto_email_report/auto_email_report.py @@ -245,6 +245,7 @@ def send_monthly(): def make_links(columns, data): for row in data: + doc_name = row.get('name') for col in columns: if col.fieldtype == "Link" and col.options != "Currency": if col.options and row.get(col.fieldname): @@ -253,8 +254,9 @@ def make_links(columns, data): if col.options and row.get(col.fieldname) and row.get(col.options): row[col.fieldname] = get_link_to_form(row[col.options], row[col.fieldname]) elif col.fieldtype == "Currency" and row.get(col.fieldname): - row[col.fieldname] = frappe.format_value(row[col.fieldname], col) - + doc = frappe.get_doc(col.parent, doc_name) if doc_name else None + # Pass the Document to get the currency based on docfield option + row[col.fieldname] = frappe.format_value(row[col.fieldname], col, doc=doc) return columns, data def update_field_types(columns): @@ -262,4 +264,4 @@ def update_field_types(columns): if col.fieldtype in ("Link", "Dynamic Link", "Currency") and col.options != "Currency": col.fieldtype = "Data" col.options = "" - return columns \ No newline at end of file + return columns From b215921a971e3b95ff6f93d63da3ffc70a10766f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 25 May 2021 21:44:54 +0530 Subject: [PATCH 32/49] fix: Convert only as_dict and debug values to bool Given the scope of its usage at this point, this becomes a problem when you'd have a field named y,n, true, false and order_by that field, or have the same values for a document name that parent parameter would accept out of all those that Frappe REST allows. --- frappe/api.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/api.py b/frappe/api.py index c69f76a755..6427cbfbd8 100644 --- a/frappe/api.py +++ b/frappe/api.py @@ -119,10 +119,11 @@ def handle(): frappe.local.form_dict.limit or frappe.local.form_dict.limit_page_length or 20, ) - # convert strings to native types - frappe.local.form_dict.update( - {x: sbool(y) for x, y in frappe.local.form_dict.items()} - ) + # convert strings to native types - only as_dict and debug accept bool + for param in ["as_dict", "debug"]: + param_val = frappe.local.form_dict.get(param) + if param_val is not None: + frappe.local.form_dict[param] = sbool(param_val) # evaluate frappe.get_list data = frappe.call(frappe.client.get_list, doctype, **frappe.local.form_dict) From 8764134309907f8c89336a90b1edb51ef1b23dff Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 26 May 2021 10:02:12 +0530 Subject: [PATCH 33/49] ci: Run mariadb tests after PR merge for coverage badge --- .github/workflows/server-mariadb-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 1742e813c6..1c7655528c 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -3,6 +3,8 @@ name: Server on: pull_request: workflow_dispatch: + push: + branches: [ develop ] jobs: test: From b0f1bbc37812daa2f4212401374e8317b200750c Mon Sep 17 00:00:00 2001 From: Akshay Kumar Tripathi <50769001+AkshayKumarTripathi@users.noreply.github.com> Date: Wed, 26 May 2021 10:37:50 +0530 Subject: [PATCH 34/49] fix: corrected the function get_url (#13330) * Removed /Form from the function get_url As per issue #12820, I think /Form was causing a problem so I removed it. Now the get_url returns URL in the format: "app/doctype/name". * fix: Change to f-strings * Implement slug on get_url * Removed slug for names --- frappe/model/document.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 623916597e..a3f8ad0cfa 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -17,6 +17,7 @@ from frappe.model.workflow import set_workflow_state_on_action from frappe.utils.global_search import update_global_search from frappe.integrations.doctype.webhook import run_webhooks from frappe.desk.form.document_follow import follow_document +from frappe.desk.utils import slug from frappe.core.doctype.server_script.server_script_utils import run_server_script_for_doc_event # once_only validation @@ -1202,8 +1203,8 @@ class Document(BaseDocument): doc.set(fieldname, flt(doc.get(fieldname), self.precision(fieldname, doc.parentfield))) def get_url(self): - """Returns Desk URL for this document. `/app/Form/{doctype}/{name}`""" - return "/app/Form/{doctype}/{name}".format(doctype=self.doctype, name=self.name) + """Returns Desk URL for this document. `/app/{doctype}/{name}`""" + return f"/app/{slug(self.doctype)}/{self.name}" def add_comment(self, comment_type='Comment', text=None, comment_email=None, link_doctype=None, link_name=None, comment_by=None): """Add a comment to this document. From 71632073ecbb88996ce355a353715eba02b50304 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 26 May 2021 15:45:02 +0530 Subject: [PATCH 35/49] chore: Fix status badge link --- README.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e00bea7857..11343a632a 100644 --- a/README.md +++ b/README.md @@ -14,18 +14,21 @@ From ab9b6bf36ec142fe4fc3bb47b46c9455fb4fa8b1 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 26 May 2021 15:46:41 +0530 Subject: [PATCH 36/49] ci: Run ui tests after PR merge for status badge --- .github/workflows/ui-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index d9ccb07da0..f2f43f10f8 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -3,6 +3,8 @@ name: UI on: pull_request: workflow_dispatch: + push: + branches: [ develop ] jobs: test: From 2ccea73aa28af560ebc7eae31c9f49e0a891937f Mon Sep 17 00:00:00 2001 From: Krishna Kant Hati Date: Thu, 27 May 2021 01:52:28 +0530 Subject: [PATCH 37/49] fix: tooltip displays correct title --- frappe/public/js/frappe/form/controls/link.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 43bd7443ab..af92f3b7a9 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -200,10 +200,11 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat if(frappe.model.can_create(doctype)) { // new item r.results.push({ - label: "" + html: "" + " " + __("Create a new {0}", [__(me.get_options())]) + "", + label: __("Create a new {0}", [__(me.get_options())]), value: "create_new__link_option", action: me.new_doc }); @@ -213,10 +214,11 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat if (locals && locals['DocType']) { // not applicable in web forms r.results.push({ - label: "" + html: "" + " " + __("Advanced Search") + "", + label: __("Advanced Search"), value: "advanced_search__link_option", action: me.open_advanced_search }); From 23406d031a7b6f849424f12203cf20537d94ff2e Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 27 May 2021 11:50:56 +0530 Subject: [PATCH 38/49] fix: Store assets.json directly in assets folder assets.json stores assets of all apps, so doesn't make sense to put it in frappe folder --- esbuild/esbuild.js | 7 +------ frappe/build.py | 2 +- frappe/utils/__init__.py | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index ecf0d49511..5154adb634 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -343,12 +343,7 @@ async function write_assets_json(metafile) { } } - let assets_json_path = path.resolve( - assets_path, - "frappe", - "dist", - "assets.json" - ); + let assets_json_path = path.resolve(assets_path, "assets.json"); let assets_json; try { assets_json = await fs.promises.readFile(assets_json_path, "utf-8"); diff --git a/frappe/build.py b/frappe/build.py index c970ae3a28..1df42ca2e6 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -50,7 +50,7 @@ def build_missing_files(): development = frappe.local.conf.developer_mode or frappe.local.dev_server build_mode = "development" if development else "production" - assets_json = frappe.read_file(frappe.get_app_path('frappe', 'public', 'dist', 'assets.json')) + assets_json = frappe.read_file("assets/assets.json") if assets_json: assets_json = frappe.parse_json(assets_json) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 436dcba028..1470616ca9 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -809,7 +809,7 @@ def get_assets_json(): assets_json = None if not assets_json: - assets_json = frappe.read_file("assets/frappe/dist/assets.json") + assets_json = frappe.read_file("assets/assets.json") cache.set_value("assets_json", assets_json, shared=True) frappe.local.assets_json = frappe.safe_decode(assets_json) From 38efba692db8461df920bff5102b5ed9ab0b5b31 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 28 May 2021 08:44:41 +0530 Subject: [PATCH 39/49] fix: Conditionally set css variables --- .../website_theme/website_theme_template.scss | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/frappe/website/doctype/website_theme/website_theme_template.scss b/frappe/website/doctype/website_theme/website_theme_template.scss index 2a67baf541..34cd66a4fe 100644 --- a/frappe/website/doctype/website_theme/website_theme_template.scss +++ b/frappe/website/doctype/website_theme/website_theme_template.scss @@ -1,13 +1,13 @@ {% if google_font %} @import url("https://fonts.googleapis.com/css2?family={{ google_font.replace(' ', '+') }}:{{ font_properties }}&display=swap"); $font-family-sans-serif: "{{ google_font }}", -apple-system, BlinkMacSystemFont, - "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", - "Droid Sans", "Helvetica Neue", sans-serif; + "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", + "Droid Sans", "Helvetica Neue", sans-serif; {% endif -%} {% if primary_color %}$primary: {{ frappe.db.get_value('Color', primary_color, 'color') }};{% endif -%} {% if dark_color %}$dark: {{ frappe.db.get_value('Color', dark_color, 'color') }};{% endif -%} -{% if text_color %}$body-color: {{ frappe.db.get_value('Color', text_color, 'color') }};{% endif -%} +{% if text_color %}$body-text-color: {{ frappe.db.get_value('Color', text_color, 'color') }};{% endif -%} {% if background_color %}$body-bg: {{ frappe.db.get_value('Color', background_color, 'color') }};{% endif -%} $enable-shadows: {{ button_shadows and "true" or "false" }}; @@ -24,7 +24,7 @@ $enable-rounded: {{ button_rounded_corners and "true" or "false" }}; {% if font_size -%} body { - font-size: {{ font_size }}; + font-size: {{ font_size }}; } {%- endif %} @@ -32,12 +32,16 @@ body { {{ custom_scss or '' }} :root { + {% if primary_color %} --primary: #{$primary}; --primary-color: #{$primary}; - --primary-light: #{$primary-light}; - --light: #{$light}; + {% endif -%} + {% if background_color %} --bg-color: #{$body-bg}; - --text-color: #{$body-color}; - --text-light: #{$body-color}; + {% endif -%} + {% if text_color %} + --text-color: #{$body-text-color}; + --text-light: #{$body-text-color}; + {% endif -%} } From a4402868774af0ac1721fd48520cd12c158e0eb7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 28 May 2021 09:14:27 +0530 Subject: [PATCH 40/49] fix: check if salutation already exists in email body (backport #13196) (#13358) --- frappe/public/js/frappe/views/communication.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index f65f1a83c2..e2aaec553d 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -724,9 +724,14 @@ frappe.views.CommunicationComposer = class { } message += await this.get_signature(); - if (this.real_name && !message.includes("")) { - message = `

${__('Dear')} ${this.real_name},

-
${message}`; + + const SALUTATION_END_COMMENT = ""; + if (this.real_name && !message.includes(SALUTATION_END_COMMENT)) { + this.message = ` +

${__('Dear')} ${this.real_name},

+ ${SALUTATION_END_COMMENT}
+ ${message} + `; } if (this.is_a_reply) { From f14d9bd3dade7c09577b65c6b8a75495b53285c8 Mon Sep 17 00:00:00 2001 From: leela Date: Fri, 28 May 2021 07:50:31 +0530 Subject: [PATCH 41/49] fix: refresh form section while refreshing the field If all the fields with in a section has depends_on property and in such case section itself stays hidden as all fields with in it are hidden. Currently when any form field is updated, we are refreshing only the fields but not the sections. So, field refresh is not affecting the form because of section being hidden. Fix is to refresh the sections when ever form fields are refreshed. (cherry picked from commit 20c6e24a9f58a303b36da8cce247d273e6e3582f) --- frappe/public/js/frappe/form/form.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 88d7ceaa94..b7d375be8e 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -175,6 +175,7 @@ frappe.ui.form.Form = class FrappeForm { field && ["Link", "Dynamic Link"].includes(field.df.fieldtype) && field.validate && field.validate(value); me.layout.refresh_dependency(); + me.layout.refresh_sections(); let object = me.script_manager.trigger(fieldname, doc.doctype, doc.name); return object; } @@ -1078,6 +1079,7 @@ frappe.ui.form.Form = class FrappeForm { if (this.fields_dict[fname] && this.fields_dict[fname].refresh) { this.fields_dict[fname].refresh(); this.layout.refresh_dependency(); + this.layout.refresh_sections(); } } From 4ce8a600e8c607ba418b0dd79265f917518d57a6 Mon Sep 17 00:00:00 2001 From: leela Date: Mon, 31 May 2021 09:02:29 +0530 Subject: [PATCH 42/49] fix: Tests and sider issues --- .../doctype/email_account/email_account.py | 8 +++---- .../email_account/test_email_account.py | 22 +++++++++---------- frappe/email/receive.py | 2 -- frappe/tests/test_email.py | 5 ++--- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index eea2dfc73e..6ee106bb12 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -19,7 +19,7 @@ from frappe.utils import (validate_email_address, cint, cstr, get_datetime, from frappe.utils.user import is_system_user from frappe.utils.jinja import render_template from frappe.email.smtp import SMTPServer -from frappe.email.receive import EmailServer, Email, InboundMail, SentEmailInInboxError +from frappe.email.receive import EmailServer, InboundMail, SentEmailInInboxError from poplib import error_proto from dateutil.relativedelta import relativedelta from datetime import datetime, timedelta @@ -469,12 +469,12 @@ class EmailAccount(Document): """retrive and return inbound mails. """ - if not self.enable_incoming: - return [] - if frappe.local.flags.in_test: return [InboundMail(msg, self) for msg in test_mails or []] + if not self.enable_incoming: + return [] + email_sync_rule = self.build_email_sync_rule() try: email_server = self.get_incoming_server(in_receive=True, email_sync_rule=email_sync_rule) diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index 7bf49593f4..35cacac45a 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -1,23 +1,23 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -from __future__ import unicode_literals -from frappe.core.doctype import communication -from frappe.core.doctype.communication.communication import Communication +import os +import email +import unittest +from datetime import datetime, timedelta + from frappe.email.receive import InboundMail, SentEmailInInboxError, Email from frappe.email.email_body import get_message_id -import frappe, os -import unittest, email - +import frappe from frappe.test_runner import make_test_records +from frappe.core.doctype.communication.email import make +from frappe.desk.form.load import get_attachments +from frappe.email.doctype.email_account.email_account import notify_unreplied make_test_records("User") make_test_records("Email Account") -from frappe.core.doctype.communication.email import make -from frappe.desk.form.load import get_attachments -from frappe.email.doctype.email_account.email_account import notify_unreplied -from datetime import datetime, timedelta + class TestEmailAccount(unittest.TestCase): @classmethod @@ -240,7 +240,7 @@ class TestInboundMail(unittest.TestCase): email_account = frappe.get_doc("Email Account", "_Test Email Account 1") email_account.db_set("enable_incoming", 0) - def tearDown(self): + def setUp(self): cleanup() frappe.db.sql('delete from `tabEmail Queue`') frappe.db.sql('delete from `tabToDo`') diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 52b9110ddb..7b3623df21 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -821,7 +821,6 @@ class InboundMail(Email): @staticmethod def get_doc(doctype, docname, ignore_error=False): try: - print(doctype, docname) return frappe.get_doc(doctype, docname) except frappe.DoesNotExistError: if ignore_error: @@ -862,7 +861,6 @@ class InboundMail(Email): for field in email_fields: if hasattr(meta, field): fields[field] = getattr(meta, field) - print("FIELDS::::", fields) return fields @staticmethod diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index ff7e6d534c..a37d1a519c 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -1,8 +1,6 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt -from __future__ import unicode_literals - import unittest, frappe, re, email from six import PY3 @@ -176,7 +174,8 @@ class TestEmail(unittest.TestCase): frappe.db.sql('''delete from `tabCommunication` where sender = 'sukh@yyy.com' ''') with open(frappe.get_app_path('frappe', 'tests', 'data', 'email_with_image.txt'), 'r') as raw: - communication = email_account.insert_communication(raw.read()) + mails = email_account.get_inbound_mails(test_mails=[raw.read()]) + communication = mails[0].process() self.assertTrue(re.search(''']*src=["']/private/files/rtco1.png[^>]*>''', communication.content)) self.assertTrue(re.search(''']*src=["']/private/files/rtco2.png[^>]*>''', communication.content)) From 8aff94d514fe7cc898f407620764d07642d8a30e Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Mon, 31 May 2021 10:30:26 +0530 Subject: [PATCH 43/49] fix(jinja): Remove frappe.utils.jinja.get_jenv() from jinja globals --- frappe/utils/jinja_globals.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/utils/jinja_globals.py b/frappe/utils/jinja_globals.py index 347d52dc57..2881a21aff 100644 --- a/frappe/utils/jinja_globals.py +++ b/frappe/utils/jinja_globals.py @@ -2,7 +2,6 @@ # MIT License. See license.txt from __future__ import unicode_literals -from frappe.utils.jinja import get_jenv def resolve_class(classes): @@ -22,6 +21,8 @@ def resolve_class(classes): def inspect(var, render=True): + from frappe.utils.jinja import get_jenv + context = {"var": var} if render: html = "
{{ var | pprint | e }}
" From e73efaec04b86d41eb4a773288fd2ab73aa327a4 Mon Sep 17 00:00:00 2001 From: Saqib Date: Mon, 31 May 2021 10:42:50 +0530 Subject: [PATCH 44/49] fix: Return promise while reloading doc (#13219) --- frappe/public/js/frappe/form/form.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index b7d375be8e..35ebf9274d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1069,7 +1069,7 @@ frappe.ui.form.Form = class FrappeForm { if(!this.doc.__islocal) { frappe.model.remove_from_locals(this.doctype, this.docname); - frappe.model.with_doc(this.doctype, this.docname, () => { + return frappe.model.with_doc(this.doctype, this.docname, () => { this.refresh(); }); } From 77590184929bba29d08faf9823feaa0e32f55b46 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 31 May 2021 16:26:38 +0530 Subject: [PATCH 45/49] chore(deps): bump ws from 7.4.2 to 7.4.6 (#13369) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 86719d81f4..298c424b72 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7608,9 +7608,9 @@ write-file-atomic@^3.0.0: typedarray-to-buffer "^3.1.5" ws@~7.4.2: - version "7.4.2" - resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.2.tgz#782100048e54eb36fe9843363ab1c68672b261dd" - integrity sha512-T4tewALS3+qsrpGI/8dqNMLIVdq/g/85U98HPMa6F0m6xTbvhXU6RCQLqPH3+SlomNV/LdY6RXEbBpMH6EOJnA== + version "7.4.6" + resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.6.tgz#5654ca8ecdeee47c33a9a4bf6d28e2be2980377c" + integrity sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A== xdg-basedir@^4.0.0: version "4.0.0" From 872299b3f3846be0c115c12bf74789b556cb2942 Mon Sep 17 00:00:00 2001 From: Fisher Yu <12823863+szufisher@users.noreply.github.com> Date: Mon, 31 May 2021 19:01:42 +0800 Subject: [PATCH 46/49] fix: Translate Strings (#13277) --- frappe/core/doctype/data_import/data_import.js | 2 +- frappe/core/doctype/doctype/doctype.js | 4 ++-- frappe/custom/doctype/customize_form/customize_form.js | 2 +- frappe/public/js/frappe/data_import/data_exporter.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 079bdaa09c..216db53c72 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -91,7 +91,7 @@ frappe.ui.form.on('Data Import', { if (frm.doc.status.includes('Success')) { frm.add_custom_button( - __('Go to {0} List', [frm.doc.reference_doctype]), + __('Go to {0} List', [__(frm.doc.reference_doctype)]), () => frappe.set_route('List', frm.doc.reference_doctype) ); } diff --git a/frappe/core/doctype/doctype/doctype.js b/frappe/core/doctype/doctype/doctype.js index 1a173f7252..b4d3fb9a89 100644 --- a/frappe/core/doctype/doctype/doctype.js +++ b/frappe/core/doctype/doctype/doctype.js @@ -33,11 +33,11 @@ frappe.ui.form.on('DocType', { if (!frm.is_new() && !frm.doc.istable) { if (frm.doc.issingle) { - frm.add_custom_button(__('Go to {0}', [frm.doc.name]), () => { + frm.add_custom_button(__('Go to {0}', [__(frm.doc.name)]), () => { window.open(`/app/${frappe.router.slug(frm.doc.name)}`); }); } else { - frm.add_custom_button(__('Go to {0} List', [frm.doc.name]), () => { + frm.add_custom_button(__('Go to {0} List', [__(frm.doc.name)]), () => { window.open(`/app/${frappe.router.slug(frm.doc.name)}`); }); } diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index d9d8ae196e..4e00456f0d 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -117,7 +117,7 @@ frappe.ui.form.on("Customize Form", { frappe.customize_form.set_primary_action(frm); frm.add_custom_button( - __("Go to {0} List", [frm.doc.doc_type]), + __("Go to {0} List", [__(frm.doc.doc_type)]), function() { frappe.set_route("List", frm.doc.doc_type); }, diff --git a/frappe/public/js/frappe/data_import/data_exporter.js b/frappe/public/js/frappe/data_import/data_exporter.js index dee4839b34..03e6288856 100644 --- a/frappe/public/js/frappe/data_import/data_exporter.js +++ b/frappe/public/js/frappe/data_import/data_exporter.js @@ -72,8 +72,8 @@ frappe.data_import.DataExporter = class DataExporter { let child_fieldname = df.fieldname; let label = df.reqd ? // prettier-ignore - __('{0} ({1}) (1 row mandatory)', [df.label || df.fieldname, doctype]) - : __('{0} ({1})', [df.label || df.fieldname, doctype]); + __('{0} ({1}) (1 row mandatory)', [__(df.label || df.fieldname), __(doctype)]) + : __('{0} ({1})', [__(df.label || df.fieldname), __(doctype)]); return { label, fieldname: child_fieldname, From 0afe774ddb1e8129a031ca136a3fff9d71dee5be Mon Sep 17 00:00:00 2001 From: getsali <30821137+getsali@users.noreply.github.com> Date: Mon, 31 May 2021 16:32:58 +0530 Subject: [PATCH 47/49] fix: Select appropriate email template response (#13051) * Update email_group.py Fix: Select appropriate email template response, Welcome email template can be either html or rich text * refactor: Simpify code Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/email/doctype/email_group/email_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/email/doctype/email_group/email_group.py b/frappe/email/doctype/email_group/email_group.py index b19a134713..b1c1295f75 100755 --- a/frappe/email/doctype/email_group/email_group.py +++ b/frappe/email/doctype/email_group/email_group.py @@ -105,6 +105,6 @@ def send_welcome_email(welcome_email, email, email_group): email=email, email_group=email_group ) - - message = frappe.render_template(welcome_email.response, args) + email_message = welcome_email.response or welcome_email.response_html + message = frappe.render_template(email_message, args) frappe.sendmail(email, subject=welcome_email.subject, message=message) From c033e0d34d9f257e8e8242e17bfbba43e24f1256 Mon Sep 17 00:00:00 2001 From: Aditya Hase Date: Mon, 31 May 2021 19:24:36 +0530 Subject: [PATCH 48/49] fix(server scripts): Restrict access to python's internal attributes --- frappe/__init__.py | 17 +++++++++++++++++ frappe/utils/safe_exec.py | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/frappe/__init__.py b/frappe/__init__.py index 9b208f7c2d..5793b224a3 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1693,6 +1693,23 @@ def safe_eval(code, eval_globals=None, eval_locals=None): "round": round } + UNSAFE_ATTRIBUTES = { + # Generator Attributes + "gi_frame", "gi_code", + # Coroutine Attributes + "cr_frame", "cr_code", "cr_origin", + # Async Generator Attributes + "ag_code", "ag_frame", + # Traceback Attributes + "tb_frame", "tb_next", + # Format Attributes + "format", "format_map", + } + + for attribute in UNSAFE_ATTRIBUTES: + if attribute in code: + throw('Illegal rule {0}. Cannot use "{1}"'.format(bold(code), attribute)) + if '__' in code: throw('Illegal rule {0}. Cannot use "__"'.format(bold(code))) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index d1307a3a90..632fc5df23 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -150,6 +150,7 @@ def get_safe_globals(): # default writer allows write access out._write_ = _write out._getitem_ = _getitem + out._getattr_ = _getattr # allow iterators and list comprehension out._getiter_ = iter @@ -176,6 +177,27 @@ def _getitem(obj, key): raise SyntaxError('Key starts with _') return obj[key] +def _getattr(object, name, default=None): + # guard function for RestrictedPython + # allow any key to be accessed as long as + # 1. it does not start with an underscore (safer_getattr) + # 2. it is not an UNSAFE_ATTRIBUTES + + UNSAFE_ATTRIBUTES = { + # Generator Attributes + "gi_frame", "gi_code", + # Coroutine Attributes + "cr_frame", "cr_code", "cr_origin", + # Async Generator Attributes + "ag_code", "ag_frame", + # Traceback Attributes + "tb_frame", "tb_next", + } + + if isinstance(name, str) and (name in UNSAFE_ATTRIBUTES): + raise SyntaxError("{name} is an unsafe attribute".format(name=name)) + return RestrictedPython.Guards.safer_getattr(object, name, default=default) + def _write(obj): # guard function for RestrictedPython # allow writing to any object From 39eee3e5c8fbd514ebedbd0526eb13508250336d Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Jun 2021 19:02:01 +0530 Subject: [PATCH 49/49] fix: Add min-width for awesomplete class --- frappe/public/scss/common/awesomeplete.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/public/scss/common/awesomeplete.scss b/frappe/public/scss/common/awesomeplete.scss index 096da1e2fd..619d6a9e3e 100644 --- a/frappe/public/scss/common/awesomeplete.scss +++ b/frappe/public/scss/common/awesomeplete.scss @@ -31,6 +31,7 @@ margin: 0; padding: var(--padding-xs); z-index: 1; + min-width: 250px; &> li { cursor: pointer;