From c2c858f70e8aabf5c23b8922130b98d7d4e22fb7 Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Thu, 24 Feb 2022 09:49:43 +0530 Subject: [PATCH 01/38] fix: clean up logs job was broken --- frappe/core/doctype/activity_log/activity_log.py | 3 ++- frappe/core/doctype/log_settings/log_settings.py | 3 ++- frappe/email/queue.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 69565a2c2a..92a23dcc04 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -48,6 +48,7 @@ def clear_activity_logs(days=None): if not days: days = 90 doctype = DocType("Activity Log") + duration = (Now() - Interval(days=days)) frappe.db.delete(doctype, filters=( - doctype.creation < PseudoColumn(f"({Now() - Interval(days=days)})") + doctype.creation < duration )) \ No newline at end of file diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 5c9bc6c265..8dcf5f4ade 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -18,8 +18,9 @@ class LogSettings(Document): def clear_error_logs(self): table = DocType("Error Log") + duration = (Now() - Interval(days=self.clear_error_log_after)) frappe.db.delete(table, filters=( - table.creation < PseudoColumn(f"({Now() - Interval(days=self.clear_error_log_after)})") + table.creation < duration )) def clear_activity_logs(self): diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 16e3fecf48..79c9aa7e03 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -170,7 +170,7 @@ def clear_outbox(days=None): days=31 email_queues = frappe.db.sql_list("""SELECT `name` FROM `tabEmail Queue` - WHERE `priority`=0 AND `modified` < (NOW() - INTERVAL '{0}' DAY)""".format(days)) + WHERE `modified` < (NOW() - INTERVAL '{0}' DAY)""".format(days)) if email_queues: frappe.db.delete("Email Queue", {"name": ("in", email_queues)}) From 75583bf692f84c4b74ce5088c1191602c044ff0c Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Thu, 24 Feb 2022 18:02:15 +0530 Subject: [PATCH 02/38] fix: removed redundant pieces & rewrote the query with qb --- frappe/core/doctype/activity_log/activity_log.py | 3 +-- frappe/core/doctype/log_settings/log_settings.py | 4 +--- frappe/email/queue.py | 11 +++++++++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 92a23dcc04..0a02b45d32 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -48,7 +48,6 @@ def clear_activity_logs(days=None): if not days: days = 90 doctype = DocType("Activity Log") - duration = (Now() - Interval(days=days)) frappe.db.delete(doctype, filters=( - doctype.creation < duration + doctype.creation < (Now() - Interval(days=days)) )) \ No newline at end of file diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 8dcf5f4ade..dec33070c0 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -7,7 +7,6 @@ from frappe import _ from frappe.model.document import Document from frappe.query_builder import DocType, Interval from frappe.query_builder.functions import Now -from pypika.terms import PseudoColumn class LogSettings(Document): @@ -18,9 +17,8 @@ class LogSettings(Document): def clear_error_logs(self): table = DocType("Error Log") - duration = (Now() - Interval(days=self.clear_error_log_after)) frappe.db.delete(table, filters=( - table.creation < duration + table.creation < (Now() - Interval(days=self.clear_error_log_after)) )) def clear_activity_logs(self): diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 79c9aa7e03..629b23b601 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -5,6 +5,8 @@ import frappe from frappe import msgprint, _ from frappe.utils.verified_command import get_signed_params, verify_request from frappe.utils import get_url, now_datetime, cint +from frappe.query_builder import DocType, Interval +from frappe.query_builder.functions import Now def get_emails_sent_this_month(email_account=None): """Get count of emails sent from a specific email account. @@ -169,8 +171,13 @@ def clear_outbox(days=None): if not days: days=31 - email_queues = frappe.db.sql_list("""SELECT `name` FROM `tabEmail Queue` - WHERE `modified` < (NOW() - INTERVAL '{0}' DAY)""".format(days)) + email_queue = DocType("Email Queue") + queues = (frappe.qb.from_(email_queue) + .select(email_queue.name) + .where(email_queue.modified < (Now() - Interval(days=days))) + .run(as_dict=True)) + + email_queues = [queue.name for queue in queues] if email_queues: frappe.db.delete("Email Queue", {"name": ("in", email_queues)}) From 62eca433d7816e570bdfc3fe4eb6dad245f963de Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Sat, 26 Feb 2022 11:05:52 +0530 Subject: [PATCH 03/38] fix: added test cases for log settings --- .../doctype/log_settings/test_log_settings.py | 129 +++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index 40287948fd..f3021c3e1a 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -1,8 +1,131 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2020, Frappe Technologies and Contributors +# Copyright (c) 2022, Frappe Technologies and Contributors # License: MIT. See LICENSE -# import frappe +import frappe import unittest +current = frappe.utils.now_datetime() +past = frappe.utils.add_to_date(current, days=-4) class TestLogSettings(unittest.TestCase): - pass + @classmethod + def setUpClass(cls): + fieldnames = ['clear_error_log_after', 'clear_activity_log_after', 'clear_email_queue_after'] + for fieldname in fieldnames: + frappe.set_value("Log Settings", None, fieldname, 1) + + @classmethod + def tearDownClass(cls): + if frappe.db.exists({"doctype": "Activity Log", "subject": "Test subject"}): + activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') + for log in activity_logs: + frappe.db.delete("Activity Log", log) + frappe.db.commit() + + if frappe.db.exists({"doctype": "Email Queue", "expose_recipients": "test@receiver.com"}): + email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients='test@receiver.com'), pluck='name') + for queue in email_queues: + frappe.db.delete("Email Queue", queue) + frappe.db.commit() + + if frappe.db.exists({"doctype": "Error Log", "method": "test_method"}): + error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') + for log in error_logs: + frappe.db.delete("Error Log", log) + frappe.db.commit() + + def test_create_activity_logs(self): + doc1 = frappe.get_doc({ + "doctype": "Activity Log", + "subject": "Test subject", + "full_name": "test user1", + }) + doc1.insert(ignore_permissions=True) + + #creation can't be set while inserting new_doc + frappe.db.set_value("Activity Log", doc1.name, "creation", past) + + doc2 = frappe.get_doc({ + "doctype": "Activity Log", + "subject": "Test subject", + "full_name": "test user2", + "creation": current + }) + doc2.insert(ignore_permissions=True) + + activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') + + self.assertEqual(len(activity_logs), 2) + + def test_create_error_logs(self): + traceback = """ + Traceback (most recent call last): + File "apps/frappe/frappe/email/doctype/email_account/email_account.py", line 489, in get_inbound_mails + messages = email_server.get_messages() + File "apps/frappe/frappe/email/receive.py", line 166, in get_messages + if self.has_login_limit_exceeded(e): + File "apps/frappe/frappe/email/receive.py", line 315, in has_login_limit_exceeded + return "-ERR Exceeded the login limit" in strip(cstr(e.message)) + AttributeError: 'AttributeError' object has no attribute 'message' + """ + doc1 = frappe.get_doc({ + "doctype": "Error Log", + "method": "test_method", + "error": traceback, + "creation": past + }) + doc1.insert(ignore_permissions=True) + + frappe.db.set_value("Error Log", doc1.name, "creation", past) + + doc2 = frappe.get_doc({ + "doctype": "Error Log", + "method": "test_method", + "error": traceback, + "creation": current + }) + doc2.insert(ignore_permissions=True) + + error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') + self.assertEqual(len(error_logs), 2) + + def test_create_email_queue(self): + doc1 = frappe.get_doc({ + "doctype": "Email Queue", + "sender": "test1@example.com", + "message": "This is a test email1", + "priority": 1, + "expose_recipients": "test@receiver.com", + }) + doc1.insert(ignore_permissions=True) + + frappe.db.set_value("Email Queue", doc1.name, "creation", past) + frappe.db.set_value("Email Queue", doc1.name, "modified", past, update_modified=False) + + doc2 = frappe.get_doc({ + "doctype": "Email Queue", + "sender": "test2@example.com", + "message": "This is a test email2", + "priority": 1, + "expose_recipients": "test@receiver.com", + "creation": current + }) + doc2.insert(ignore_permissions=True) + + email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients="test@receiver.com"), pluck='name') + + self.assertEqual(len(email_queues), 2) + + def test_delete_logs(self): + from frappe.core.doctype.log_settings.log_settings import run_log_clean_up + + run_log_clean_up() + + activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') + self.assertEqual(len(activity_logs), 1) + + error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') + self.assertEqual(len(error_logs), 1) + + email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients='test@receiver.com'), pluck='name') + self.assertEqual(len(email_queues), 1) + From 05a658c86ed243cd4c61a4b0b3fabc38c349de3d Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Sat, 26 Feb 2022 11:08:36 +0530 Subject: [PATCH 04/38] chore: fix linter issues --- frappe/core/doctype/log_settings/test_log_settings.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index f3021c3e1a..60ee75e5dc 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -19,19 +19,16 @@ class TestLogSettings(unittest.TestCase): activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') for log in activity_logs: frappe.db.delete("Activity Log", log) - frappe.db.commit() if frappe.db.exists({"doctype": "Email Queue", "expose_recipients": "test@receiver.com"}): email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients='test@receiver.com'), pluck='name') for queue in email_queues: frappe.db.delete("Email Queue", queue) - frappe.db.commit() if frappe.db.exists({"doctype": "Error Log", "method": "test_method"}): error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') for log in error_logs: frappe.db.delete("Error Log", log) - frappe.db.commit() def test_create_activity_logs(self): doc1 = frappe.get_doc({ From 91b220c37c6fcd84e91285beff515aca8935f3c2 Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Sat, 26 Feb 2022 12:18:08 +0530 Subject: [PATCH 05/38] fix: added proper messages --- frappe/custom/doctype/customize_form/customize_form.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index 4862185b99..75e3f8a274 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -134,7 +134,7 @@ frappe.ui.form.on("Customize Form", { ); frm.add_custom_button( - __("Reset to defaults"), + __("Reset Property Setters"), function() { frappe.customize_form.confirm( __("Remove all customizations?"), @@ -315,9 +315,9 @@ frappe.customize_form.confirm = function(msg, frm) { if (!frm.doc.doc_type) return; var d = new frappe.ui.Dialog({ - title: 'Reset To Defaults', + title: 'Reset Property Setters', fields: [ - {fieldtype:"HTML", options:__("All customizations will be removed. Please confirm.")}, + {fieldtype:"HTML", options:__("All property setters will be removed. Please confirm.")}, ], primary_action: function() { return frm.call({ @@ -328,7 +328,7 @@ frappe.customize_form.confirm = function(msg, frm) { frappe.msgprint(r.exc); } else { d.hide(); - frappe.show_alert({message:__('Customizations Reset'), indicator:'green'}); + frappe.show_alert({message:__('Property Setters Reset'), indicator:'green'}); frappe.customize_form.clear_locals_and_refresh(frm); } } From d76eca36fe3e2b87a412767b7eb345fcae774d95 Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Mon, 28 Feb 2022 13:52:15 +0530 Subject: [PATCH 06/38] fix: remove custom fields as well on reset to default --- .../custom/doctype/customize_form/customize_form.js | 8 ++++---- .../custom/doctype/customize_form/customize_form.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index 75e3f8a274..4862185b99 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -134,7 +134,7 @@ frappe.ui.form.on("Customize Form", { ); frm.add_custom_button( - __("Reset Property Setters"), + __("Reset to defaults"), function() { frappe.customize_form.confirm( __("Remove all customizations?"), @@ -315,9 +315,9 @@ frappe.customize_form.confirm = function(msg, frm) { if (!frm.doc.doc_type) return; var d = new frappe.ui.Dialog({ - title: 'Reset Property Setters', + title: 'Reset To Defaults', fields: [ - {fieldtype:"HTML", options:__("All property setters will be removed. Please confirm.")}, + {fieldtype:"HTML", options:__("All customizations will be removed. Please confirm.")}, ], primary_action: function() { return frm.call({ @@ -328,7 +328,7 @@ frappe.customize_form.confirm = function(msg, frm) { frappe.msgprint(r.exc); } else { d.hide(); - frappe.show_alert({message:__('Property Setters Reset'), indicator:'green'}); + frappe.show_alert({message:__('Customizations Reset'), indicator:'green'}); frappe.customize_form.clear_locals_and_refresh(frm); } } diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 92a540447f..13474fdbdf 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -487,12 +487,21 @@ def reset_customization(doctype): setters = frappe.get_all("Property Setter", filters={ 'doc_type': doctype, 'field_name': ['!=', 'naming_series'], - 'property': ['!=', 'options'] + 'property': ['!=', 'options'], + 'owner': ['!=', 'Administrator'] }, pluck='name') for setter in setters: frappe.delete_doc("Property Setter", setter) + custom_fields = frappe.get_all("Custom Field", filters={ + 'dt': doctype, + 'owner': ['!=', 'Administrator'] + }, pluck='name') + + for field in custom_fields: + frappe.delete_doc("Custom Field", field) + frappe.clear_cache(doctype=doctype) doctype_properties = { From 54fe7d7ea0d2bb5fa4bb5cdf748088ba2c5a3d03 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Mar 2022 08:56:51 +0530 Subject: [PATCH 07/38] feat: Add a flag to identify system generated customization --- frappe/custom/doctype/custom_field/custom_field.json | 10 +++++++++- .../doctype/property_setter/property_setter.json | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/frappe/custom/doctype/custom_field/custom_field.json b/frappe/custom/doctype/custom_field/custom_field.json index f09829a688..5632da2149 100644 --- a/frappe/custom/doctype/custom_field/custom_field.json +++ b/frappe/custom/doctype/custom_field/custom_field.json @@ -7,6 +7,7 @@ "document_type": "Setup", "engine": "InnoDB", "field_order": [ + "is_system_generated", "dt", "module", "label", @@ -425,13 +426,20 @@ "fieldtype": "Link", "label": "Module (for export)", "options": "Module Def" + }, + { + "default": "0", + "fieldname": "is_system_generated", + "fieldtype": "Check", + "label": "Is System Generated", + "read_only": 1 } ], "icon": "fa fa-glass", "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2022-02-14 15:42:21.885999", + "modified": "2022-02-28 22:22:54.893269", "modified_by": "Administrator", "module": "Custom", "name": "Custom Field", diff --git a/frappe/custom/doctype/property_setter/property_setter.json b/frappe/custom/doctype/property_setter/property_setter.json index 9707f1ee1c..039826b3b7 100644 --- a/frappe/custom/doctype/property_setter/property_setter.json +++ b/frappe/custom/doctype/property_setter/property_setter.json @@ -6,6 +6,7 @@ "document_type": "Setup", "engine": "InnoDB", "field_order": [ + "is_system_generated", "help", "sb0", "doctype_or_field", @@ -103,13 +104,20 @@ { "fieldname": "section_break_9", "fieldtype": "Section Break" + }, + { + "default": "0", + "fieldname": "is_system_generated", + "fieldtype": "Check", + "label": "Is System Generated", + "read_only": 1 } ], "icon": "fa fa-glass", "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-12-14 14:15:41.929071", + "modified": "2022-02-28 22:24:12.377693", "modified_by": "Administrator", "module": "Custom", "name": "Property Setter", From c51a581e2ce73fb45afa1b87d8c960032edf005e Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Mar 2022 09:20:47 +0530 Subject: [PATCH 08/38] feat: Set `is_system_generated` as false if customization is created via Customize Form --- frappe/__init__.py | 3 ++- frappe/custom/doctype/custom_field/custom_field.py | 5 ++--- frappe/custom/doctype/customize_form/customize_form.js | 3 ++- frappe/custom/doctype/customize_form/customize_form.py | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 8a8b70afe3..52efb4069d 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1250,7 +1250,7 @@ def get_newargs(fn, kwargs): return newargs -def make_property_setter(args, ignore_validate=False, validate_fields_for_doctype=True): +def make_property_setter(args, ignore_validate=False, validate_fields_for_doctype=True, is_system_generated=True): """Create a new **Property Setter** (for overriding DocType and DocField properties). If doctype is not specified, it will create a property setter for all fields with the @@ -1281,6 +1281,7 @@ def make_property_setter(args, ignore_validate=False, validate_fields_for_doctyp 'property': args.property, 'value': args.value, 'property_type': args.property_type or "Data", + 'is_system_generated': is_system_generated, '__islocal': 1 }) ps.flags.ignore_validate = ignore_validate diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index cb1ea2c54d..af10c6d76a 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -119,7 +119,7 @@ def create_custom_field_if_values_exist(doctype, df): frappe.db.count(dt=doctype, filters=IfNull(df.fieldname, "") != ""): create_custom_field(doctype, df) -def create_custom_field(doctype, df, ignore_validate=False): +def create_custom_field(doctype, df, ignore_validate=False, is_system_generated=True): df = frappe._dict(df) if not df.fieldname and df.label: df.fieldname = frappe.scrub(df.label) @@ -130,8 +130,7 @@ def create_custom_field(doctype, df, ignore_validate=False): "permlevel": 0, "fieldtype": 'Data', "hidden": 0, - # Looks like we always use this programatically? - # "is_standard": 1 + "is_system_generated": is_system_generated }) custom_field.update(df) custom_field.flags.ignore_validate = ignore_validate diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index 4862185b99..e81ef1f089 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -243,7 +243,8 @@ frappe.ui.form.on("Customize Form Field", { }, fields_add: function(frm, cdt, cdn) { var f = frappe.model.get_doc(cdt, cdn); - f.is_custom_field = 1; + f.is_system_generated = false; + f.is_custom_field = true; } }); diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 81cd38ff87..efee006301 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -402,7 +402,7 @@ class CustomizeForm(Document): "property": prop, "value": value, "property_type": property_type - }) + }, is_system_generated=False) def get_existing_property_value(self, property_name, fieldname=None): # check if there is any need to make property setter! From 952e7d26c406500da2a76368569a9ccc2be95b2e Mon Sep 17 00:00:00 2001 From: hrwx Date: Tue, 8 Mar 2022 15:36:21 +0000 Subject: [PATCH 09/38] feat: toggle cameras for uploading --- .../js/frappe/file_uploader/FileUploader.vue | 12 +++-- frappe/public/js/frappe/ui/capture.js | 53 +++++++++++++++---- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/frappe/public/js/frappe/file_uploader/FileUploader.vue b/frappe/public/js/frappe/file_uploader/FileUploader.vue index 1b30726a7a..7922341af2 100644 --- a/frappe/public/js/frappe/file_uploader/FileUploader.vue +++ b/frappe/public/js/frappe/file_uploader/FileUploader.vue @@ -526,11 +526,13 @@ export default { error: true }); capture.show(); - capture.submit(data_url => { - let filename = `capture_${frappe.datetime.now_datetime().replaceAll(/[: -]/g, '_')}.png`; - this.url_to_file(data_url, filename, 'image/png').then((file) => - this.add_files([file]) - ); + capture.submit(images => { + images.forEach(data_url => { + let filename = `capture_${frappe.datetime.now_datetime().replaceAll(/[: -]/g, '_')}.png`; + this.url_to_file(data_url, filename, 'image/png').then((file) => + this.add_files([file]) + ); + }); }); }, show_google_drive_picker() { diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index d408fadb33..e91ab6788b 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -45,6 +45,9 @@ frappe.ui.Capture = class { constructor(options = {}) { this.options = frappe.ui.Capture.OPTIONS; this.set_options(options); + + this.facing_mode = "environment"; + this.images = [] } set_options(options) { @@ -54,7 +57,13 @@ frappe.ui.Capture = class { } render() { - return navigator.mediaDevices.getUserMedia({ video: true }).then(stream => { + let constraints = { + video: { + facingMode: this.facing_mode + } + } + + return navigator.mediaDevices.getUserMedia(constraints).then(stream => { this.stream = stream; this.dialog = new frappe.ui.Dialog({ @@ -70,33 +79,55 @@ frappe.ui.Capture = class { const set_take_photo_action = () => { this.dialog.set_primary_action(__('Take Photo'), () => { const data_url = frappe._.get_data_uri(video); - $e.find('.fc-p').attr('src', data_url); + $e.find('.fc-preview').attr('src', data_url); - $e.find('.fc-s').hide(); - $e.find('.fc-p').show(); + this.images.push(data_url); + + $e.find('.fc-stream').hide(); + $e.find('.fc-preview').show(); this.dialog.set_secondary_action_label(__('Retake')); this.dialog.get_secondary_btn().show(); + this.dialog.custom_actions.find(".btn-multiple").show(); this.dialog.set_primary_action(__('Submit'), () => { this.hide(); - if (this.callback) this.callback(data_url); + if (this.callback) this.callback(this.images); }); }); + + this.dialog.set_secondary_action_label(__('Switch Camera')); + this.dialog.set_secondary_action(() => { + this.facing_mode = this.facing_mode == "environment" ? "user" : "environment"; + frappe.show_alert({ + message: __("Switching Camera") + }); + this.hide(); + this.show(); + }); }; set_take_photo_action(); this.dialog.set_secondary_action(() => { - $e.find('.fc-p').hide(); - $e.find('.fc-s').show(); + this.images.pop(); + $e.find('.fc-preview').hide(); + $e.find('.fc-stream').show(); - this.dialog.get_secondary_btn().hide(); this.dialog.get_primary_btn().off('click'); set_take_photo_action(); }); - this.dialog.get_secondary_btn().hide(); + this.dialog.add_custom_action(__("Take Multiple"), () => { + $e.find('.fc-preview').hide(); + $e.find('.fc-stream').show(); + + this.dialog.get_primary_btn().off('click'); + this.dialog.custom_actions.find(".btn-multiple").hide(); + set_take_photo_action(); + }, "btn-multiple"); + + this.dialog.custom_actions.find(".btn-multiple").hide(); const $e = $(frappe.ui.Capture.TEMPLATE); @@ -150,8 +181,8 @@ frappe.ui.Capture.TEMPLATE = `
- - + +
From 3ca2a1b801eb2806225cb951ae2746ed554cdae0 Mon Sep 17 00:00:00 2001 From: Himanshu Date: Tue, 8 Mar 2022 16:34:14 +0000 Subject: [PATCH 10/38] chore: sider fixes --- frappe/public/js/frappe/ui/capture.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index e91ab6788b..7bf107e679 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -47,7 +47,7 @@ frappe.ui.Capture = class { this.set_options(options); this.facing_mode = "environment"; - this.images = [] + this.images = []; } set_options(options) { @@ -61,7 +61,7 @@ frappe.ui.Capture = class { video: { facingMode: this.facing_mode } - } + }; return navigator.mediaDevices.getUserMedia(constraints).then(stream => { this.stream = stream; From e18ca59dc64e44aa815770ba92e0ad34c93c01c6 Mon Sep 17 00:00:00 2001 From: hrwx Date: Wed, 9 Mar 2022 19:35:40 +0000 Subject: [PATCH 11/38] fix: improve ux --- frappe/public/js/frappe/ui/capture.js | 250 ++++++++++++++++-------- frappe/public/scss/common/controls.scss | 7 + 2 files changed, 176 insertions(+), 81 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index e91ab6788b..d1d6f9b518 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -47,7 +47,7 @@ frappe.ui.Capture = class { this.set_options(options); this.facing_mode = "environment"; - this.images = [] + this.images = []; } set_options(options) { @@ -56,7 +56,44 @@ frappe.ui.Capture = class { return this; } - render() { + show() { + let me = this; + + this.dialog = new frappe.ui.Dialog({ + title: this.options.title, + animate: this.options.animate, + fields: [ + { + fieldtype: "HTML", + fieldname: "capture" + }, + { + fieldtype: "HTML", + fieldname: "total_count" + } + ], + on_hide: this.stop_media_stream() + }); + + this.dialog.get_close_btn().on('click', () => { + me.hide(); + }); + + this.render_stream() + .then(() => { + me.dialog.show(); + }) + .catch(err => { + if (me.options.error) { + frappe.show_alert(frappe.ui.Capture.ERR_MESSAGE, 3); + } + + throw err; + }); + } + + render_stream() { + let me = this; let constraints = { video: { facingMode: this.facing_mode @@ -64,94 +101,145 @@ frappe.ui.Capture = class { } return navigator.mediaDevices.getUserMedia(constraints).then(stream => { - this.stream = stream; + me.stream = stream; + me.dialog.custom_actions.empty(); - this.dialog = new frappe.ui.Dialog({ - title: this.options.title, - animate: this.options.animate, - on_hide: () => this.stop_media_stream() - }); + me.setup_take_photo_action(); + me.setup_preview_action(); + me.setup_toggle_camera(); - this.dialog.get_close_btn().on('click', () => { - this.hide(); - }); + me.$template = $(frappe.ui.Capture.TEMPLATE); - const set_take_photo_action = () => { - this.dialog.set_primary_action(__('Take Photo'), () => { - const data_url = frappe._.get_data_uri(video); - $e.find('.fc-preview').attr('src', data_url); - - this.images.push(data_url); - - $e.find('.fc-stream').hide(); - $e.find('.fc-preview').show(); - - this.dialog.set_secondary_action_label(__('Retake')); - this.dialog.get_secondary_btn().show(); - this.dialog.custom_actions.find(".btn-multiple").show(); - - this.dialog.set_primary_action(__('Submit'), () => { - this.hide(); - if (this.callback) this.callback(this.images); - }); - }); - - this.dialog.set_secondary_action_label(__('Switch Camera')); - this.dialog.set_secondary_action(() => { - this.facing_mode = this.facing_mode == "environment" ? "user" : "environment"; - frappe.show_alert({ - message: __("Switching Camera") - }); - this.hide(); - this.show(); - }); - }; - - set_take_photo_action(); - - this.dialog.set_secondary_action(() => { - this.images.pop(); - $e.find('.fc-preview').hide(); - $e.find('.fc-stream').show(); - - this.dialog.get_primary_btn().off('click'); - set_take_photo_action(); - }); + me.video = me.$template.find('video')[0]; + me.video.srcObject = me.stream; + me.video.play(); - this.dialog.add_custom_action(__("Take Multiple"), () => { - $e.find('.fc-preview').hide(); - $e.find('.fc-stream').show(); + let field = me.dialog.get_field("capture"); + $(field.wrapper).html(me.$template); + }); + } - this.dialog.get_primary_btn().off('click'); - this.dialog.custom_actions.find(".btn-multiple").hide(); - set_take_photo_action(); - }, "btn-multiple"); + render_preview() { + this.$template.find('.fc-stream-container').hide(); + this.$template.find('.fc-preview-container').show(); + + let images = ``; + + this.images.forEach((image, idx) => { + images += ` +
+ + ${frappe.utils.icon("close", "lg")} + + +
+ `; + }); - this.dialog.custom_actions.find(".btn-multiple").hide(); + this.$template.find('.fc-preview-container').empty(); + $(this.$template.find('.fc-preview-container')).html( + `
+ ${images} +
` + ); + + this.setup_capture_action(); + this.setup_submit_action(); + this.setup_remove_action(); + this.update_count(); + this.dialog.custom_actions.empty(); + } - const $e = $(frappe.ui.Capture.TEMPLATE); + setup_take_photo_action() { + let me = this; - const video = $e.find('video')[0]; - video.srcObject = this.stream; - video.play(); - const $container = $(this.dialog.body); + this.dialog.set_primary_action(__('Take Photo'), () => { + const data_url = frappe._.get_data_uri(me.video); - $container.html($e); + me.images.push(data_url); + me.setup_preview_action(); + me.update_count(); }); } - show() { - this.render() - .then(() => { - this.dialog.show(); - }) - .catch(err => { - if (this.options.error) { - frappe.show_alert(frappe.ui.Capture.ERR_MESSAGE, 3); - } + setup_preview_action() { + let me = this; - throw err; + if (!this.images.length) { + return; + } + + this.dialog.set_secondary_action_label(__("Preview")); + this.dialog.set_secondary_action(() => { + me.dialog.get_primary_btn().off('click'); + me.render_preview(); + }); + } + + setup_remove_action() { + let me = this; + let elements = this.$template[0].getElementsByClassName("capture-remove-btn"); + + elements.forEach(el => { + el.onclick = () => { + let idx = parseInt(el.getAttribute("data-idx")) + + me.images.splice(idx, 1); + me.render_preview(); + } + }); + } + + update_count() { + let field = this.dialog.get_field("total_count"); + let msg = `${__("Total Images")}: ${this.images.length}`; + + if (this.images.length === 0) { + msg = __("No Images"); + } + + $(field.wrapper).html(` +
+
${msg}
+
+ `); + } + + setup_toggle_camera() { + let me = this; + + this.dialog.add_custom_action(__("Switch Camera"), () => { + me.facing_mode = me.facing_mode == "environment" ? "user" : "environment"; + + frappe.show_alert({ + message: __("Switching Camera") }); + + me.stop_media_stream(); + me.render_stream(); + }, "btn-switch"); + } + + setup_capture_action() { + let me = this; + + this.dialog.set_secondary_action_label(__("Capture")); + this.dialog.set_secondary_action(() => { + me.dialog.get_primary_btn().off('click'); + me.render_stream(); + }); + } + + setup_submit_action() { + let me = this; + + this.dialog.set_primary_action(__('Submit'), () => { + me.hide(); + + if (me.callback) { + me.callback(me.images); + } + }); } hide() { @@ -179,11 +267,11 @@ frappe.ui.Capture.OPTIONS = { frappe.ui.Capture.ERR_MESSAGE = __('Unable to load camera.'); frappe.ui.Capture.TEMPLATE = `
-
-
- - -
+
+ +
+
`; diff --git a/frappe/public/scss/common/controls.scss b/frappe/public/scss/common/controls.scss index 954916c911..fcc924650e 100644 --- a/frappe/public/scss/common/controls.scss +++ b/frappe/public/scss/common/controls.scss @@ -460,3 +460,10 @@ button.data-pill { justify-content: space-between; align-items: center; } + +.capture-remove-btn { + position: absolute; + top: 0; + right: 0; + cursor: pointer; +} \ No newline at end of file From 7f4f14558939e1859bba9a231d9a0d4dc60e8921 Mon Sep 17 00:00:00 2001 From: hrwx Date: Wed, 9 Mar 2022 21:36:10 +0000 Subject: [PATCH 12/38] fix: stream not being visible --- frappe/public/js/frappe/ui/capture.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index df93cbedf2..727a5762f1 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -103,7 +103,7 @@ frappe.ui.Capture = class { return navigator.mediaDevices.getUserMedia(constraints).then(stream => { me.stream = stream; me.dialog.custom_actions.empty(); - + me.dialog.get_primary_btn().off('click'); me.setup_take_photo_action(); me.setup_preview_action(); me.setup_toggle_camera(); @@ -112,6 +112,7 @@ frappe.ui.Capture = class { me.video = me.$template.find('video')[0]; me.video.srcObject = me.stream; + me.video.load(); me.video.play(); let field = me.dialog.get_field("capture"); @@ -120,14 +121,16 @@ frappe.ui.Capture = class { } render_preview() { + this.stop_media_stream(); this.$template.find('.fc-stream-container').hide(); this.$template.find('.fc-preview-container').show(); + this.dialog.get_primary_btn().off('click'); let images = ``; this.images.forEach((image, idx) => { images += ` -
+
${frappe.utils.icon("close", "lg")} @@ -225,7 +228,6 @@ frappe.ui.Capture = class { this.dialog.set_secondary_action_label(__("Capture")); this.dialog.set_secondary_action(() => { - me.dialog.get_primary_btn().off('click'); me.render_stream(); }); } From 538e6c8ad552a20aa1af15d5b6b2639770c2f74c Mon Sep 17 00:00:00 2001 From: hrwx Date: Wed, 9 Mar 2022 23:59:19 +0000 Subject: [PATCH 13/38] chore: sider fixes --- frappe/public/js/frappe/ui/capture.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index 727a5762f1..41358ea596 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -185,11 +185,11 @@ frappe.ui.Capture = class { elements.forEach(el => { el.onclick = () => { - let idx = parseInt(el.getAttribute("data-idx")) + let idx = parseInt(el.getAttribute("data-idx")); me.images.splice(idx, 1); me.render_preview(); - } + }; }); } From 191803e15fedd14cfd961f0006e38f6f76c93e82 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 17 Mar 2022 12:49:35 +0530 Subject: [PATCH 14/38] fix: Add a patch to set is_system_generated flag --- frappe/patches.txt | 1 + .../v14_0/update_is_system_generated_flag.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 frappe/patches/v14_0/update_is_system_generated_flag.py diff --git a/frappe/patches.txt b/frappe/patches.txt index 0d2a6162c2..23d14e4cba 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -196,3 +196,4 @@ frappe.patches.v14_0.copy_mail_data #08.03.21 frappe.patches.v14_0.update_github_endpoints #08-11-2021 frappe.patches.v14_0.remove_db_aggregation frappe.patches.v14_0.update_color_names_in_kanban_board_column +frappe.patches.v14_0.update_is_system_generated_flag diff --git a/frappe/patches/v14_0/update_is_system_generated_flag.py b/frappe/patches/v14_0/update_is_system_generated_flag.py new file mode 100644 index 0000000000..657e02aebc --- /dev/null +++ b/frappe/patches/v14_0/update_is_system_generated_flag.py @@ -0,0 +1,17 @@ +import frappe + +def execute(): + # assuming all customization generated by Admin is system generated customization + custom_field = frappe.qb.DocType("Custom Field") + ( + frappe.qb.update(custom_field) + .set(custom_field.is_system_generated, True) + .where(custom_field.owner == 'Administrator').run() + ) + + property_setter = frappe.qb.DocType("Property Setter") + ( + frappe.qb.update(property_setter) + .set(property_setter.is_system_generated, True) + .where(property_setter.owner == 'Administrator').run() + ) From 0a94d025a277ee1a7190aed37bd5c769f4de328c Mon Sep 17 00:00:00 2001 From: hrwx Date: Sat, 19 Mar 2022 23:32:30 +0000 Subject: [PATCH 15/38] feat: use mobile native camera --- frappe/public/js/frappe/ui/capture.js | 68 ++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 7 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index 41358ea596..8fb61fa669 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -28,6 +28,24 @@ frappe._.get_data_uri = element => { return data_uri; }; +function get_file_input() { + let input = document.createElement("input"); + input.setAttribute("type", "file"); + input.setAttribute("accept", "image/*"); + input.setAttribute("multiple", ""); + + return input; +}; + +function read(file) { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result); + reader.onerror = reject + reader.readAsDataURL(file); + }); +}; + /** * @description Frappe's Capture object. * @@ -57,8 +75,16 @@ frappe.ui.Capture = class { } show() { - let me = this; + this.build_dialog(); + + if (frappe.is_mobile()) { + this.show_for_mobile(); + } else { + this.show_for_desktop(); + } + } + build_dialog() { this.dialog = new frappe.ui.Dialog({ title: this.options.title, animate: this.options.animate, @@ -75,9 +101,36 @@ frappe.ui.Capture = class { on_hide: this.stop_media_stream() }); + this.$template = $(frappe.ui.Capture.TEMPLATE); + + let field = this.dialog.get_field("capture"); + $(field.wrapper).html(this.$template); + this.dialog.get_close_btn().on('click', () => { me.hide(); }); + } + + show_for_mobile() { + let me = this; + if (!me.input) { + me.input = get_file_input(); + } + + me.input.onchange = async () => { + for (let file of me.input.files) { + let f = await read(file) + me.images.push(f); + } + + me.render_preview(); + me.dialog.show(); + }; + me.input.click(); + } + + show_for_desktop() { + let me = this; this.render_stream() .then(() => { @@ -108,15 +161,12 @@ frappe.ui.Capture = class { me.setup_preview_action(); me.setup_toggle_camera(); - me.$template = $(frappe.ui.Capture.TEMPLATE); - + me.$template.find('.fc-stream-container').show(); + me.$template.find('.fc-preview-container').hide(); me.video = me.$template.find('video')[0]; me.video.srcObject = me.stream; me.video.load(); me.video.play(); - - let field = me.dialog.get_field("capture"); - $(field.wrapper).html(me.$template); }); } @@ -228,7 +278,11 @@ frappe.ui.Capture = class { this.dialog.set_secondary_action_label(__("Capture")); this.dialog.set_secondary_action(() => { - me.render_stream(); + if (frappe.is_mobile()) { + me.show_for_mobile(); + } else { + me.render_stream(); + } }); } From e5b32c326b00c9c10f81835f34972517c5ee6c60 Mon Sep 17 00:00:00 2001 From: hrwx Date: Sat, 19 Mar 2022 23:39:44 +0000 Subject: [PATCH 16/38] chore: sider fixes --- frappe/public/js/frappe/ui/capture.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/ui/capture.js b/frappe/public/js/frappe/ui/capture.js index 8fb61fa669..eb444be4e9 100644 --- a/frappe/public/js/frappe/ui/capture.js +++ b/frappe/public/js/frappe/ui/capture.js @@ -35,16 +35,16 @@ function get_file_input() { input.setAttribute("multiple", ""); return input; -}; +} function read(file) { return new Promise((resolve, reject) => { const reader = new FileReader(); reader.onload = () => resolve(reader.result); - reader.onerror = reject + reader.onerror = reject; reader.readAsDataURL(file); }); -}; +} /** * @description Frappe's Capture object. @@ -85,7 +85,8 @@ frappe.ui.Capture = class { } build_dialog() { - this.dialog = new frappe.ui.Dialog({ + let me = this; + me.dialog = new frappe.ui.Dialog({ title: this.options.title, animate: this.options.animate, fields: [ @@ -101,12 +102,12 @@ frappe.ui.Capture = class { on_hide: this.stop_media_stream() }); - this.$template = $(frappe.ui.Capture.TEMPLATE); + me.$template = $(frappe.ui.Capture.TEMPLATE); - let field = this.dialog.get_field("capture"); - $(field.wrapper).html(this.$template); + let field = me.dialog.get_field("capture"); + $(field.wrapper).html(me.$template); - this.dialog.get_close_btn().on('click', () => { + me.dialog.get_close_btn().on('click', () => { me.hide(); }); } @@ -119,7 +120,7 @@ frappe.ui.Capture = class { me.input.onchange = async () => { for (let file of me.input.files) { - let f = await read(file) + let f = await read(file); me.images.push(f); } From 1934340a1b81313b191381206c3dc6aa3fdedac1 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 20 Mar 2022 01:46:27 +0100 Subject: [PATCH 17/38] refactor: don't assign variable to itself --- frappe/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) mode change 100755 => 100644 frappe/handler.py diff --git a/frappe/handler.py b/frappe/handler.py old mode 100755 new mode 100644 index 3fd1c096e4..07c7d7a30c --- a/frappe/handler.py +++ b/frappe/handler.py @@ -250,7 +250,7 @@ def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): try: args = json.loads(args) except ValueError: - args = args + pass method_obj = getattr(doc, method) fn = getattr(method_obj, '__func__', method_obj) From f650408daa0da57d0ce9072eb4a5b2f244b0e4bd Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 20 Mar 2022 18:02:44 +0100 Subject: [PATCH 18/38] refactor: use frappe.parse_json --- frappe/handler.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frappe/handler.py b/frappe/handler.py index 07c7d7a30c..ebc72da937 100644 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -225,11 +225,10 @@ def ping(): def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): """run a whitelisted controller method""" - import json - import inspect + from inspect import getfullargspec - if not args: - args = arg or "" + if not args and arg: + args = arg if dt: # not called from a doctype (from a page) if not dn: @@ -237,9 +236,7 @@ def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): doc = frappe.get_doc(dt, dn) else: - if isinstance(docs, str): - docs = json.loads(docs) - + docs = frappe.parse_json(docs) doc = frappe.get_doc(docs) doc._original_modified = doc.modified doc.check_if_latest() @@ -248,7 +245,7 @@ def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): throw_permission_error() try: - args = json.loads(args) + args = frappe.parse_json(args) except ValueError: pass @@ -257,7 +254,7 @@ def run_doc_method(method, docs=None, dt=None, dn=None, arg=None, args=None): is_whitelisted(fn) is_valid_http_method(fn) - fnargs = inspect.getfullargspec(method_obj).args + fnargs = getfullargspec(method_obj).args if not fnargs or (len(fnargs)==1 and fnargs[0]=="self"): response = doc.run_method(method) From aa0d10fe0ecc4df770f2f77a4e05db7880384ed8 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 21 Mar 2022 17:24:18 +0530 Subject: [PATCH 19/38] fix: First set in model then save attachment --- frappe/public/js/frappe/form/controls/attach.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/attach.js b/frappe/public/js/frappe/form/controls/attach.js index bd66225171..01af3bbc89 100644 --- a/frappe/public/js/frappe/form/controls/attach.js +++ b/frappe/public/js/frappe/form/controls/attach.js @@ -110,9 +110,9 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro return this.value || null; } - on_upload_complete(attachment) { + async on_upload_complete(attachment) { if(this.frm) { - this.parse_validate_and_set_in_model(attachment.file_url); + await this.parse_validate_and_set_in_model(attachment.file_url); this.frm.attachments.update_attachment(attachment); this.frm.doc.docstatus == 1 ? this.frm.save('Update') : this.frm.save(); } From 512c62248769ce3a92cf433ded2bb53db58f281c Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 21 Mar 2022 20:51:35 +0100 Subject: [PATCH 20/38] test: make sure `exists` doesn't eat the doctype key --- frappe/tests/test_db.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 19b683aa75..10c601db00 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -327,7 +327,13 @@ class TestDB(unittest.TestCase): self.assertEqual(frappe.db.exists(dt, dn, cache=True), dn) self.assertEqual(frappe.db.exists(dt, dn), dn) self.assertEqual(frappe.db.exists(dt, {"name": ("=", dn)}), dn) - self.assertEqual(frappe.db.exists({"doctype": dt, "name": ("like", "Admin%")}), dn) + + filters = {"doctype": dt, "name": ("like", "Admin%")} + self.assertEqual(frappe.db.exists(filters), dn) + self.assertEqual( + filters["doctype"], dt + ) # make sure that doctype was not removed from filters + self.assertEqual(frappe.db.exists(dt, [["name", "=", dn]]), dn) From 44a7c0dd9318d984b20ae95180f5cf35eb146c28 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 21 Mar 2022 20:51:59 +0100 Subject: [PATCH 21/38] fix: copy dict before popping keys --- frappe/database/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index b1b5abffca..82a7e6f919 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -919,8 +919,8 @@ class Database(object): return dn if isinstance(dt, dict): - _dt = dt.pop("doctype") - dt, dn = _dt, dt + dt = dt.copy() # don't modify the original dict + dt, dn = dt.pop("doctype"), dt return self.get_value(dt, dn, ignore=True, cache=cache) From 179c9f117c7ecf8c9c2882ecf192031331294bbf Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 21 Mar 2022 21:12:05 +0100 Subject: [PATCH 22/38] perf: exists is already called in delete_doc --- frappe/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 86f8be35ea..80dd2f5f15 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -978,8 +978,7 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa def delete_doc_if_exists(doctype, name, force=0): """Delete document if exists.""" - if db.exists(doctype, name): - delete_doc(doctype, name, force=force) + delete_doc(doctype, name, force=force, ignore_missing=True) def reload_doctype(doctype, force=False, reset_permissions=False): """Reload DocType from model (`[module]/[doctype]/[name]/[name].json`) files.""" From 8cf2bf89534039b5b23a32177b469c1ddb1bd521 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 21 Mar 2022 23:09:58 +0100 Subject: [PATCH 23/38] refactor: call getfullargspec only once --- frappe/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 86f8be35ea..1501fda81a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1252,9 +1252,10 @@ def get_newargs(fn, kwargs): if hasattr(fn, 'fnargs'): fnargs = fn.fnargs else: - fnargs = inspect.getfullargspec(fn).args - fnargs.extend(inspect.getfullargspec(fn).kwonlyargs) - varkw = inspect.getfullargspec(fn).varkw + fullargspec = inspect.getfullargspec(fn) + fnargs = fullargspec.args + fnargs.extend(fullargspec.kwonlyargs) + varkw = fullargspec.varkw newargs = {} for a in kwargs: From d30f9e1d78b5e2e02ae9b7afedccd5451ab3d94f Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Tue, 22 Mar 2022 11:23:43 +0530 Subject: [PATCH 24/38] fix: wait until attach is cleared before saving --- frappe/public/js/frappe/form/controls/attach.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/attach.js b/frappe/public/js/frappe/form/controls/attach.js index 01af3bbc89..a91058a208 100644 --- a/frappe/public/js/frappe/form/controls/attach.js +++ b/frappe/public/js/frappe/form/controls/attach.js @@ -37,8 +37,8 @@ frappe.ui.form.ControlAttach = class ControlAttach extends frappe.ui.form.Contro if(this.frm) { me.parse_validate_and_set_in_model(null); me.refresh(); - me.frm.attachments.remove_attachment_by_filename(me.value, function() { - me.parse_validate_and_set_in_model(null); + me.frm.attachments.remove_attachment_by_filename(me.value, async () => { + await me.parse_validate_and_set_in_model(null); me.refresh(); me.frm.doc.docstatus == 1 ? me.frm.save('Update') : me.frm.save(); }); From c53e6d822de15d414301ee00a10b13af8f840c4a Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Fri, 18 Feb 2022 21:39:00 +0530 Subject: [PATCH 25/38] feat: parse app name from tags and urls --- frappe/exceptions.py | 3 +++ frappe/installer.py | 53 +++++++++++++++++++++++++++++++++++++--- frappe/utils/__init__.py | 5 ++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 6ee72b5f81..fcac349708 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -110,3 +110,6 @@ class InvalidAuthorizationPrefix(CSRFTokenError): pass class InvalidAuthorizationToken(CSRFTokenError): pass class InvalidDatabaseFile(ValidationError): pass class ExecutableNotFound(FileNotFoundError): pass + +class InvalidRemoteException(Exception): + pass diff --git a/frappe/installer.py b/frappe/installer.py index d10dc78286..9bb2d9993f 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -5,10 +5,11 @@ import json import os import sys from collections import OrderedDict -from typing import List, Dict +from typing import List, Dict, Tuple import frappe from frappe.defaults import _clear_cache +from frappe.utils import is_git_url def _new_site( @@ -34,7 +35,6 @@ def _new_site( from frappe.commands.scheduler import _is_scheduler_enabled from frappe.utils import get_site_path, scheduler, touch_file - if not force and os.path.exists(site): print("Site {0} already exists".format(site)) sys.exit(1) @@ -124,6 +124,52 @@ def install_db(root_login=None, root_password=None, db_name=None, source_sql=Non frappe.flags.in_install_db = False +def find_org(org_repo: str) -> Tuple[str, str]: + from frappe.exceptions import InvalidRemoteException + import requests + + org_repo = org_repo[0] + + for org in ["frappe", "erpnext"]: + res = requests.head(f"https://api.github.com/repos/{org}/{org_repo}") + if res.ok: + return org, org_repo + + raise InvalidRemoteException + + +def fetch_details_from_tag(_tag: str) -> Tuple[str, str, str]: + app_tag = _tag.split("@") + org_repo = app_tag[0].split("/") + + try: + repo, tag = app_tag + except ValueError: + repo, tag = app_tag + [None] + + try: + org, repo = org_repo + except Exception: + org, repo = find_org(org_repo) + + return org, repo, tag + + +def parse_app_name(name: str): + name = name.rstrip("/") + if os.path.exists(name): + repo = os.path.split(name)[-1] + elif is_git_url(name): + if name.startswith("git@") or name.startswith("ssh://"): + _repo = name.split(":")[1].rsplit("/", 1)[1] + else: + _repo = name.rsplit("/", 2)[2] + repo = _repo.split(".")[0] + else: + _, repo, _ = fetch_details_from_tag(name) + return repo + + def install_app(name, verbose=False, set_as_patched=True): from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs from frappe.model.sync import sync_for @@ -140,7 +186,8 @@ def install_app(name, verbose=False, set_as_patched=True): # install pre-requisites if app_hooks.required_apps: for app in app_hooks.required_apps: - install_app(app, verbose=verbose) + name = parse_app_name(name) + install_app(name, verbose=verbose) frappe.flags.in_install = name frappe.clear_cache() diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index c361b5b430..4a6d578a9c 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -918,3 +918,8 @@ def add_user_info(user, user_info): email = info.email, time_zone = info.time_zone ) + +def is_git_url(url: str) -> bool: + # modified to allow without the tailing .git from https://github.com/jonschlinkert/is-git-url.git + pattern = r"(?:git|ssh|https?|\w*@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$" + return bool(re.match(pattern, url)) From f8c40985856f60d43247f8b5de66c25f3b6c240f Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Fri, 11 Mar 2022 14:44:53 +0530 Subject: [PATCH 26/38] docs: docstings and refs --- frappe/installer.py | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 9bb2d9993f..e28a942f01 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -125,11 +125,22 @@ def install_db(root_login=None, root_password=None, db_name=None, source_sql=Non def find_org(org_repo: str) -> Tuple[str, str]: + """ find the org a repo is in + + find_org() + ref -> https://github.com/frappe/bench/blob/develop/bench/utils/__init__.py#L390 + + :param org_repo: + :type org_repo: str + + :raises InvalidRemoteException: if the org is not found + + :return: organisation and repository + :rtype: Tuple[str, str] + """ from frappe.exceptions import InvalidRemoteException import requests - org_repo = org_repo[0] - for org in ["frappe", "erpnext"]: res = requests.head(f"https://api.github.com/repos/{org}/{org_repo}") if res.ok: @@ -139,6 +150,17 @@ def find_org(org_repo: str) -> Tuple[str, str]: def fetch_details_from_tag(_tag: str) -> Tuple[str, str, str]: + """ parse org, repo, tag from string + + fetch_details_from_tag() + ref -> https://github.com/frappe/bench/blob/develop/bench/utils/__init__.py#L403 + + :param _tag: input string + :type _tag: str + + :return: organisation, repostitory, tag + :rtype: Tuple[str, str, str] + """ app_tag = _tag.split("@") org_repo = app_tag[0].split("/") @@ -150,12 +172,24 @@ def fetch_details_from_tag(_tag: str) -> Tuple[str, str, str]: try: org, repo = org_repo except Exception: - org, repo = find_org(org_repo) + org, repo = find_org(org_repo[0]) return org, repo, tag -def parse_app_name(name: str): +def parse_app_name(name: str) -> str: + """parse repo name from name + + __setup_details_from_git() + ref -> https://github.com/frappe/bench/blob/develop/bench/app.py#L114 + + + :param name: git tag + :type name: str + + :return: repository name + :rtype: str + """ name = name.rstrip("/") if os.path.exists(name): repo = os.path.split(name)[-1] From 1507751a01172e63462b4004c94d1abf085ee0b4 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Mon, 21 Mar 2022 19:34:38 +0530 Subject: [PATCH 27/38] test: test_app_name_parser --- frappe/tests/test_utils.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 18fca9de8c..fa6b5a3820 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import io +import os import json import unittest from datetime import date, datetime, time, timedelta @@ -14,13 +15,14 @@ import pytz from PIL import Image import frappe -from frappe.utils import ceil, evaluate_filters, floor, format_timedelta +from frappe.utils import ceil, evaluate_filters, floor, format_timedelta, get_bench_path from frappe.utils import get_url, money_in_words, parse_timedelta, scrub_urls from frappe.utils import validate_email_address, validate_url from frappe.utils.data import cast, get_time, get_timedelta, nowtime, now_datetime, validate_python_code from frappe.utils.diff import _get_value_from_version, get_version_diff, version_query from frappe.utils.image import optimize_image, strip_exif_data from frappe.utils.response import json_handler +from frappe.installer import parse_app_name class TestFilters(unittest.TestCase): @@ -510,3 +512,13 @@ class TestLinkTitle(unittest.TestCase): todo.delete() user.delete() prop_setter.delete() + +class TestAppParser(unittest.TestCase): + def test_app_name_parser(self): + bench_path = get_bench_path() + frappe_app = os.path.join(bench_path, "apps", "frappe") + self.assertEqual("frappe", parse_app_name(frappe_app)) + self.assertEqual("healthcare", parse_app_name("healthcare")) + self.assertEqual("healthcare", parse_app_name("https://github.com/frappe/healthcare.git")) + self.assertEqual("healthcare", parse_app_name("git@github.com:frappe/healthcare.git")) + self.assertEqual("healthcare", parse_app_name("frappe/healthcare@develop")) From 6b6514c796dacf4c617af481ed2825a00f0dc53b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 23 Mar 2022 09:34:19 +0530 Subject: [PATCH 28/38] fix: Use calendar name as it is Do not convert route to title case since calendar names are case sensitive, and it breaks for calendar names which are all CAPs --- frappe/public/js/frappe/views/calendar/calendar.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/views/calendar/calendar.js b/frappe/public/js/frappe/views/calendar/calendar.js index 0ab5e2e7dc..2739b3dd78 100644 --- a/frappe/public/js/frappe/views/calendar/calendar.js +++ b/frappe/public/js/frappe/views/calendar/calendar.js @@ -29,7 +29,7 @@ frappe.views.CalendarView = class CalendarView extends frappe.views.ListView { .then(() => { this.page_title = __('{0} Calendar', [this.page_title]); this.calendar_settings = frappe.views.calendar[this.doctype] || {}; - this.calendar_name = frappe.utils.to_title_case(frappe.get_route()[3] || ''); + this.calendar_name = frappe.get_route()[3]; }); } @@ -72,12 +72,17 @@ frappe.views.CalendarView = class CalendarView extends frappe.views.ListView { const calendar_name = this.calendar_name; return new Promise(resolve => { - if (calendar_name === 'Default') { + if (calendar_name === 'default') { Object.assign(options, frappe.views.calendar[this.doctype]); resolve(options); } else { frappe.model.with_doc('Calendar View', calendar_name, () => { const doc = frappe.get_doc('Calendar View', calendar_name); + if (!doc) { + frappe.show_alert(__("{0} is not a valid Calendar. Redirecting to default Calendar.", [calendar_name.bold()])); + frappe.set_route("List", this.doctype, "Calendar", "default"); + return; + } Object.assign(options, { field_map: { id: "name", From 01978ffe737e0ea1d5cbd43e0b067d42f62a4d7c Mon Sep 17 00:00:00 2001 From: shadrak gurupnor Date: Wed, 23 Mar 2022 12:16:17 +0530 Subject: [PATCH 29/38] fix: remove customizations which is not system_generated --- frappe/custom/doctype/customize_form/customize_form.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 13474fdbdf..571b12ba49 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -488,7 +488,7 @@ def reset_customization(doctype): 'doc_type': doctype, 'field_name': ['!=', 'naming_series'], 'property': ['!=', 'options'], - 'owner': ['!=', 'Administrator'] + 'is_system_generated': False }, pluck='name') for setter in setters: @@ -496,7 +496,7 @@ def reset_customization(doctype): custom_fields = frappe.get_all("Custom Field", filters={ 'dt': doctype, - 'owner': ['!=', 'Administrator'] + 'is_system_generated': False }, pluck='name') for field in custom_fields: From ee355b9cd6b0efa836eba3fa6ba1db755970f319 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 13:09:36 +0530 Subject: [PATCH 30/38] test(log_settings): test_delete_logs * Refactored TestCase to test "correctly" * Got rid of earlier flaky logic * Dropped unwanted tests --- .../doctype/log_settings/test_log_settings.py | 206 ++++++++---------- 1 file changed, 92 insertions(+), 114 deletions(-) diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index 60ee75e5dc..67574314a3 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -1,128 +1,106 @@ -# -*- coding: utf-8 -*- # Copyright (c) 2022, Frappe Technologies and Contributors # License: MIT. See LICENSE -import frappe + +from datetime import datetime import unittest -current = frappe.utils.now_datetime() -past = frappe.utils.add_to_date(current, days=-4) +import frappe +from frappe.utils import now_datetime, add_to_date +from frappe.core.doctype.log_settings.log_settings import run_log_clean_up + + class TestLogSettings(unittest.TestCase): @classmethod def setUpClass(cls): - fieldnames = ['clear_error_log_after', 'clear_activity_log_after', 'clear_email_queue_after'] - for fieldname in fieldnames: - frappe.set_value("Log Settings", None, fieldname, 1) - @classmethod - def tearDownClass(cls): - if frappe.db.exists({"doctype": "Activity Log", "subject": "Test subject"}): - activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') - for log in activity_logs: - frappe.db.delete("Activity Log", log) - - if frappe.db.exists({"doctype": "Email Queue", "expose_recipients": "test@receiver.com"}): - email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients='test@receiver.com'), pluck='name') - for queue in email_queues: - frappe.db.delete("Email Queue", queue) - - if frappe.db.exists({"doctype": "Error Log", "method": "test_method"}): - error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') - for log in error_logs: - frappe.db.delete("Error Log", log) - - def test_create_activity_logs(self): - doc1 = frappe.get_doc({ - "doctype": "Activity Log", - "subject": "Test subject", - "full_name": "test user1", - }) - doc1.insert(ignore_permissions=True) - - #creation can't be set while inserting new_doc - frappe.db.set_value("Activity Log", doc1.name, "creation", past) - - doc2 = frappe.get_doc({ - "doctype": "Activity Log", - "subject": "Test subject", - "full_name": "test user2", - "creation": current - }) - doc2.insert(ignore_permissions=True) - - activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') - - self.assertEqual(len(activity_logs), 2) - - def test_create_error_logs(self): - traceback = """ - Traceback (most recent call last): - File "apps/frappe/frappe/email/doctype/email_account/email_account.py", line 489, in get_inbound_mails - messages = email_server.get_messages() - File "apps/frappe/frappe/email/receive.py", line 166, in get_messages - if self.has_login_limit_exceeded(e): - File "apps/frappe/frappe/email/receive.py", line 315, in has_login_limit_exceeded - return "-ERR Exceeded the login limit" in strip(cstr(e.message)) - AttributeError: 'AttributeError' object has no attribute 'message' - """ - doc1 = frappe.get_doc({ - "doctype": "Error Log", - "method": "test_method", - "error": traceback, - "creation": past - }) - doc1.insert(ignore_permissions=True) - - frappe.db.set_value("Error Log", doc1.name, "creation", past) - - doc2 = frappe.get_doc({ - "doctype": "Error Log", - "method": "test_method", - "error": traceback, - "creation": current - }) - doc2.insert(ignore_permissions=True) - - error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') - self.assertEqual(len(error_logs), 2) - - def test_create_email_queue(self): - doc1 = frappe.get_doc({ - "doctype": "Email Queue", - "sender": "test1@example.com", - "message": "This is a test email1", - "priority": 1, - "expose_recipients": "test@receiver.com", - }) - doc1.insert(ignore_permissions=True) - - frappe.db.set_value("Email Queue", doc1.name, "creation", past) - frappe.db.set_value("Email Queue", doc1.name, "modified", past, update_modified=False) - - doc2 = frappe.get_doc({ - "doctype": "Email Queue", - "sender": "test2@example.com", - "message": "This is a test email2", - "priority": 1, - "expose_recipients": "test@receiver.com", - "creation": current - }) - doc2.insert(ignore_permissions=True) - - email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients="test@receiver.com"), pluck='name') - - self.assertEqual(len(email_queues), 2) + cls.savepoint = "TestLogSettings" + frappe.db.savepoint(cls.savepoint) - def test_delete_logs(self): - from frappe.core.doctype.log_settings.log_settings import run_log_clean_up + frappe.db.set_single_value( + "Log Settings", + { + "clear_error_log_after": 1, + "clear_activity_log_after": 1, + "clear_email_queue_after": 1, + }, + ) - run_log_clean_up() + @classmethod + def tearDownClass(cls): + frappe.db.rollback(save_point=cls.savepoint) - activity_logs = frappe.get_all("Activity Log", filters=dict(subject='Test subject'), pluck='name') - self.assertEqual(len(activity_logs), 1) + def setUp(self) -> None: + if self._testMethodName == "test_delete_logs": + self.datetime = frappe._dict() + self.datetime.current = now_datetime() + self.datetime.past = add_to_date(self.datetime.current, days=-4) + setup_test_logs(self.datetime.past) - error_logs = frappe.get_all("Error Log", filters=dict(method='test_method'), pluck='name') - self.assertEqual(len(error_logs), 1) + def tearDown(self) -> None: + if self._testMethodName == "test_delete_logs": + del self.datetime - email_queues = frappe.get_all("Email Queue", filters=dict(expose_recipients='test@receiver.com'), pluck='name') - self.assertEqual(len(email_queues), 1) + def test_delete_logs(self): + # make sure test data is present + activity_log_count = frappe.db.count( + "Activity Log", {"creation": ("<=", self.datetime.past)} + ) + error_log_count = frappe.db.count( + "Error Log", {"creation": ("<=", self.datetime.past)} + ) + email_queue_count = frappe.db.count( + "Email Queue", {"creation": ("<=", self.datetime.past)} + ) + + self.assertNotEqual(activity_log_count, 0) + self.assertNotEqual(error_log_count, 0) + self.assertNotEqual(email_queue_count, 0) + + # run clean up job + run_log_clean_up() + # test if logs are deleted + activity_log_count = frappe.db.count( + "Activity Log", {"creation": ("<", self.datetime.past)} + ) + error_log_count = frappe.db.count( + "Error Log", {"creation": ("<", self.datetime.past)} + ) + email_queue_count = frappe.db.count( + "Email Queue", {"creation": ("<", self.datetime.past)} + ) + + self.assertEqual(activity_log_count, 0) + self.assertEqual(error_log_count, 0) + self.assertEqual(email_queue_count, 0) + + +def setup_test_logs(past: datetime) -> None: + activity_log = frappe.get_doc( + { + "doctype": "Activity Log", + "subject": "Test subject", + "full_name": "test user2", + } + ).insert(ignore_permissions=True) + activity_log.db_set("creation", past) + + error_log = frappe.get_doc( + { + "doctype": "Error Log", + "method": "test_method", + "error": "traceback", + } + ).insert(ignore_permissions=True) + error_log.db_set("creation", past) + + doc1 = frappe.get_doc( + { + "doctype": "Email Queue", + "sender": "test1@example.com", + "message": "This is a test email1", + "priority": 1, + "expose_recipients": "test@receiver.com", + } + ).insert(ignore_permissions=True) + doc1.db_set("creation", past) From 85c7057ae4a15d1d4bd0c3a04c2d3449b5fdd17a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 13:11:17 +0530 Subject: [PATCH 31/38] fix(activity_log): Remove unused import from namespace --- frappe/core/doctype/activity_log/activity_log.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/activity_log/activity_log.py b/frappe/core/doctype/activity_log/activity_log.py index 0a02b45d32..70d4ca3ffe 100644 --- a/frappe/core/doctype/activity_log/activity_log.py +++ b/frappe/core/doctype/activity_log/activity_log.py @@ -1,15 +1,14 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2017, Frappe Technologies and contributors +# Copyright (c) 2022, Frappe Technologies and contributors # License: MIT. See LICENSE +import frappe from frappe import _ -from frappe.utils import get_fullname, now -from frappe.model.document import Document from frappe.core.utils import set_timeline_doc -import frappe +from frappe.model.document import Document from frappe.query_builder import DocType, Interval from frappe.query_builder.functions import Now -from pypika.terms import PseudoColumn +from frappe.utils import get_fullname, now + class ActivityLog(Document): def before_insert(self): @@ -50,4 +49,4 @@ def clear_activity_logs(days=None): doctype = DocType("Activity Log") frappe.db.delete(doctype, filters=( doctype.creation < (Now() - Interval(days=days)) - )) \ No newline at end of file + )) From 300227ba71d4d2b88a7fee4cfb3b7fe327227e49 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 13:11:55 +0530 Subject: [PATCH 32/38] fix(set_single_value): Make value parameter optional Similar to set_value for accepting multiple columns ot be updated for the same Table through a Dict as the second positional arg Misc: Added type hints --- frappe/database/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 1251a323d3..b20ca79298 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -10,7 +10,7 @@ import re import string from contextlib import contextmanager from time import time -from typing import Dict, List, Tuple, Union +from typing import Dict, List, Optional, Tuple, Union from pypika.terms import Criterion, NullValue, PseudoColumn @@ -556,7 +556,7 @@ class Database(object): def get_list(*args, **kwargs): return frappe.get_list(*args, **kwargs) - def set_single_value(self, doctype, fieldname, value, *args, **kwargs): + def set_single_value(self, doctype: str, fieldname: Union[str, Dict], value: Optional[Union[str, int]] = None, *args, **kwargs): """Set field value of Single DocType. :param doctype: DocType of the single object From 762aeb32f09a4cf0fe87a817b0c7353ea4aca409 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 16 Feb 2022 18:14:49 +0530 Subject: [PATCH 33/38] fix: Show assignments based on allocated todo only --- frappe/desk/doctype/todo/todo.py | 3 ++- frappe/desk/form/load.py | 1 + frappe/email/doctype/email_queue/email_queue.py | 1 - frappe/tests/test_assign.py | 12 ++++++++++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index e689faafbe..3b84b94754 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -72,7 +72,8 @@ class ToDo(Document): assignments = frappe.get_all("ToDo", filters={ "reference_type": self.reference_type, "reference_name": self.reference_name, - "status": ("!=", "Cancelled") + "status": ("!=", "Cancelled"), + "allocated_to": ("is", "set") }, pluck="allocated_to") assignments.reverse() diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 0140157c9d..9abf64c2cc 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -312,6 +312,7 @@ def get_assignments(dt, dn): 'reference_type': dt, 'reference_name': dn, 'status': ('!=', 'Cancelled'), + 'allocated_to': ("is", "set") }) @frappe.whitelist() diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 9b4f3b984c..dd8623cae2 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -111,7 +111,6 @@ class EmailQueue(Document): """ Send emails to recipients. """ if not self.can_send_now(): - frappe.db.rollback() return with SendMailContext(self, is_background_task) as ctx: diff --git a/frappe/tests/test_assign.py b/frappe/tests/test_assign.py index 05bf7e2fb3..48fe4e04e5 100644 --- a/frappe/tests/test_assign.py +++ b/frappe/tests/test_assign.py @@ -4,6 +4,7 @@ import frappe, unittest import frappe.desk.form.assign_to from frappe.desk.listview import get_group_by_count from frappe.automation.doctype.assignment_rule.test_assignment_rule import make_note +from frappe.desk.form.load import get_assignments class TestAssign(unittest.TestCase): def test_assign(self): @@ -55,6 +56,17 @@ class TestAssign(unittest.TestCase): frappe.db.rollback() + def test_assignment_removal(self): + todo = frappe.get_doc({"doctype":"ToDo", "description": "test"}).insert() + if not frappe.db.exists("User", "test@example.com"): + frappe.get_doc({"doctype":"User", "email":"test@example.com", "first_name":"Test"}).insert() + + new_todo = assign(todo, "test@example.com") + + # remove assignment + frappe.db.set_value("ToDo", new_todo[0].name, "allocated_to", "") + + self.assertFalse(get_assignments("ToDo", todo.name)) def assign(doc, user): return frappe.desk.form.assign_to.add({ From b79d55c5d3e0fcc1cd4ede664ba3c0df58829400 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 13:16:57 +0530 Subject: [PATCH 34/38] refactor(minor): clear_outbox * Use pluck API instead of building dict and then accesing keys * Styled query * Added type hints --- frappe/email/queue.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 629b23b601..f6f52e79e2 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -1,4 +1,4 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import frappe @@ -164,20 +164,16 @@ def get_queue(): by priority desc, creation asc limit 500''', { 'now': now_datetime() }, as_dict=True) -def clear_outbox(days=None): +def clear_outbox(days: int = None) -> None: """Remove low priority older than 31 days in Outbox or configured in Log Settings. Note: Used separate query to avoid deadlock """ - if not days: - days=31 - + days = days or 31 email_queue = DocType("Email Queue") - queues = (frappe.qb.from_(email_queue) - .select(email_queue.name) - .where(email_queue.modified < (Now() - Interval(days=days))) - .run(as_dict=True)) - email_queues = [queue.name for queue in queues] + email_queues = frappe.qb.from_(email_queue).select(email_queue.name).where( + email_queue.modified < (Now() - Interval(days=days)) + ).run(pluck=True) if email_queues: frappe.db.delete("Email Queue", {"name": ("in", email_queues)}) From e8fdca8698f3a16f2784bef8f0c4ad653908775a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20R=C3=ADos?= Date: Wed, 23 Mar 2022 05:07:12 -0300 Subject: [PATCH 35/38] feat(logger): implement `stream_only` to use StreamHandler instead of RotatingFileHandler (#16274) * feat: implement valued parameter 'stream_only' in 'get_logger()' in order to stream logs into stderr instead rotating log files. Co-authored-by: gavin --- frappe/utils/logger.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frappe/utils/logger.py b/frappe/utils/logger.py index 617572deb7..1291abbf47 100755 --- a/frappe/utils/logger.py +++ b/frappe/utils/logger.py @@ -13,7 +13,7 @@ from frappe.utils import get_sites default_log_level = logging.DEBUG -def get_logger(module=None, with_more_info=False, allow_site=True, filter=None, max_size=100_000, file_count=20): +def get_logger(module=None, with_more_info=False, allow_site=True, filter=None, max_size=100_000, file_count=20, stream_only=False): """Application Logger for your given module Args: @@ -23,6 +23,7 @@ def get_logger(module=None, with_more_info=False, allow_site=True, filter=None, filter (function, optional): Add a filter function for your logger. Defaults to None. max_size (int, optional): Max file size of each log file in bytes. Defaults to 100_000. file_count (int, optional): Max count of log files to be retained via Log Rotation. Defaults to 20. + stream_only (bool, optional): Whether to stream logs only to stderr (True) or use log files (False). Defaults to False. Returns: : Returns a Python logger object with Site and Bench level logging capabilities. @@ -54,11 +55,14 @@ def get_logger(module=None, with_more_info=False, allow_site=True, filter=None, logger.propagate = False formatter = logging.Formatter("%(asctime)s %(levelname)s {0} %(message)s".format(module)) - handler = RotatingFileHandler(log_filename, maxBytes=max_size, backupCount=file_count) + if stream_only: + handler = logging.StreamHandler() + else: + handler = RotatingFileHandler(log_filename, maxBytes=max_size, backupCount=file_count) handler.setFormatter(formatter) logger.addHandler(handler) - if site: + if site and not stream_only: sitelog_filename = os.path.join(site, "logs", logfile) site_handler = RotatingFileHandler(sitelog_filename, maxBytes=max_size, backupCount=file_count) site_handler.setFormatter(formatter) From 22fc955e65c6c73d2ea96caa11869ae0c1938152 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 13:35:55 +0530 Subject: [PATCH 36/38] fix: Tabs > Spaces --- frappe/utils/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/logger.py b/frappe/utils/logger.py index 1291abbf47..4699eb1949 100755 --- a/frappe/utils/logger.py +++ b/frappe/utils/logger.py @@ -23,7 +23,7 @@ def get_logger(module=None, with_more_info=False, allow_site=True, filter=None, filter (function, optional): Add a filter function for your logger. Defaults to None. max_size (int, optional): Max file size of each log file in bytes. Defaults to 100_000. file_count (int, optional): Max count of log files to be retained via Log Rotation. Defaults to 20. - stream_only (bool, optional): Whether to stream logs only to stderr (True) or use log files (False). Defaults to False. + stream_only (bool, optional): Whether to stream logs only to stderr (True) or use log files (False). Defaults to False. Returns: : Returns a Python logger object with Site and Bench level logging capabilities. From 93a36092fda2813b4ecede33371a238999b1edd5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Mar 2022 15:54:09 +0530 Subject: [PATCH 37/38] fix(test): start transaction explicitly before savepoint --- frappe/core/doctype/log_settings/test_log_settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index 67574314a3..7c9c7b5067 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -12,8 +12,9 @@ from frappe.core.doctype.log_settings.log_settings import run_log_clean_up class TestLogSettings(unittest.TestCase): @classmethod def setUpClass(cls): - cls.savepoint = "TestLogSettings" + # SAVEPOINT can only be used in transaction blocks and we don't wan't to take chances + frappe.db.begin() frappe.db.savepoint(cls.savepoint) frappe.db.set_single_value( From f80a16ed14b9e1c93d54453783521fac75b55578 Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Wed, 23 Mar 2022 11:43:04 +0100 Subject: [PATCH 38/38] feat: add translated search doctypes to hooks (#16197) In `search.py` it was hardcoded that **DocType** and **Role** get translated before matching against the search text. This way, a user can type in his local language and still see correct results. This feature is useful for other DocTypes as well. The criterion would be: there is a small, fairly static number of records, so that the performance impact of translating all names first is not too bad. This PR adds a hook `translated_search_doctypes` that determines which DocType names get translated before search. I also added **Country** to `translated_search_doctypes` for frappe. The link to **Country** is frequently used in **Address**, but until now there was no way to use it in the local language. There are ~70% less Countries than DocTypes (including ERPNext), so the performance should be fine. ERPNext could, for example, add the **Gender** DocType to this hook. As there are very few genders, translating them is fast and improves the UX. Docs: https://frappeframework.com/docs/v13/user/en/python-api/hooks/edit?wiki_page_patch=b4d7c8d6fc --- frappe/desk/search.py | 8 ++++---- frappe/hooks.py | 2 ++ frappe/utils/boilerplate.py | 7 +++++++ 3 files changed, 13 insertions(+), 4 deletions(-) mode change 100755 => 100644 frappe/utils/boilerplate.py diff --git a/frappe/desk/search.py b/frappe/desk/search.py index b54ea46268..dd22f821cf 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -9,7 +9,6 @@ from frappe import _, is_whitelisted import re import wrapt -UNTRANSLATED_DOCTYPES = ["DocType", "Role"] def sanitize_searchfield(searchfield): blacklisted_keywords = ['select', 'delete', 'drop', 'update', 'case', 'and', 'or', 'like'] @@ -114,6 +113,7 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, or_filters = [] + translated_search_doctypes = frappe.get_hooks("translated_search_doctypes") # build from doctype if txt: search_fields = ["name"] @@ -125,7 +125,7 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, for f in search_fields: fmeta = meta.get_field(f.strip()) - if (doctype not in UNTRANSLATED_DOCTYPES) and (f == "name" or (fmeta and fmeta.fieldtype in ["Data", "Text", "Small Text", "Long Text", + if (doctype not in translated_search_doctypes) and (f == "name" or (fmeta and fmeta.fieldtype in ["Data", "Text", "Small Text", "Long Text", "Link", "Select", "Read Only", "Text Editor"])): or_filters.append([doctype, f.strip(), "like", "%{0}%".format(txt)]) @@ -160,7 +160,7 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, ptype = 'select' if frappe.only_has_select_perm(doctype) else 'read' ignore_permissions = True if doctype == "DocType" else (cint(ignore_user_permissions) and has_permission(doctype, ptype=ptype)) - if doctype in UNTRANSLATED_DOCTYPES: + if doctype in translated_search_doctypes: page_length = None values = frappe.get_list(doctype, @@ -175,7 +175,7 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0, as_list=not as_dict, strict=False) - if doctype in UNTRANSLATED_DOCTYPES: + if doctype in translated_search_doctypes: # Filtering the values array so that query is included in very element values = ( v for v in values diff --git a/frappe/hooks.py b/frappe/hooks.py index be1b0134c1..78f4a2d801 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -383,3 +383,5 @@ global_search_doctypes = { {"doctype": "Web Form"} ] } + +translated_search_doctypes = ["DocType", "Role", "Country", "Gender", "Salutation"] diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py old mode 100755 new mode 100644 index 634c99de13..cb9ce27a09 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -333,6 +333,13 @@ app_license = "{app_license}" # "{app_name}.auth.validate" # ] +# Translation +# -------------------------------- + +# Make link fields search translated document names for these DocTypes +# Recommended only for DocTypes which have limited documents with untranslated names +# For example: Role, Gender, etc. +# translated_search_doctypes = [] """ desktop_template = """from frappe import _