diff --git a/.mergify.yml b/.mergify.yml index 63fe1a0086..838ce75835 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -5,6 +5,7 @@ pull_request_rules: - and: - author!=surajshetty3416 - author!=gavindsouza + - author!=deepeshgarg007 - or: - base=version-13 - base=version-12 diff --git a/cypress/integration/control_dynamic_link.js b/cypress/integration/control_dynamic_link.js new file mode 100644 index 0000000000..cc1eb0b695 --- /dev/null +++ b/cypress/integration/control_dynamic_link.js @@ -0,0 +1,128 @@ +context('Dynamic Link', () => { + before(() => { + cy.login(); + cy.visit('/app/doctype'); + return cy.window().its('frappe').then(frappe => { + return frappe.xcall('frappe.tests.ui_test_helpers.create_doctype', { + name: 'Test Dynamic Link', + fields: [ + { + "label": "Document Type", + "fieldname": "doc_type", + "fieldtype": "Link", + "options": "DocType", + "in_list_view": 1, + "in_standard_filter": 1, + }, + { + "label": "Document ID", + "fieldname": "doc_id", + "fieldtype": "Dynamic Link", + "options": "doc_type", + "in_list_view": 1, + "in_standard_filter": 1, + }, + ] + }); + }); + }); + + + function get_dialog_with_dynamic_link() { + return cy.dialog({ + title: 'Dynamic Link', + fields: [{ + "label": "Document Type", + "fieldname": "doc_type", + "fieldtype": "Link", + "options": "DocType", + "in_list_view": 1, + }, + { + "label": "Document ID", + "fieldname": "doc_id", + "fieldtype": "Dynamic Link", + "options": "doc_type", + "in_list_view": 1, + }] + }); + } + + function get_dialog_with_dynamic_link_option() { + return cy.dialog({ + title: 'Dynamic Link', + fields: [{ + "label": "Document Type", + "fieldname": "doc_type", + "fieldtype": "Link", + "options": "DocType", + "in_list_view": 1, + }, + { + "label": "Document ID", + "fieldname": "doc_id", + "fieldtype": "Dynamic Link", + "get_options": () => { + return "User"; + }, + "in_list_view": 1, + }] + }); + } + + it('Creating a dynamic link by passing option as function and verifying it in a dialog', () => { + get_dialog_with_dynamic_link_option().as('dialog'); + cy.get_field('doc_type').clear(); + cy.fill_field('doc_type', 'User', 'Link'); + cy.get_field('doc_id').click(); + + //Checking if the listbox have length greater than 0 + cy.get('[data-fieldname="doc_id"]').find('.awesomplete').find("li").its('length').should('be.gte', 0); + cy.get('.btn-modal-close').click({force: true}); + }); + + it('Creating a dynamic link and verifying it in a dialog', () => { + get_dialog_with_dynamic_link().as('dialog'); + cy.get_field('doc_type').clear(); + cy.fill_field('doc_type', 'User', 'Link'); + cy.get_field('doc_id').click(); + + //Checking if the listbox have length greater than 0 + cy.get('[data-fieldname="doc_id"]').find('.awesomplete').find("li").its('length').should('be.gte', 0); + cy.get('.btn-modal-close').click({force: true, multiple: true}); + }); + + it('Creating a dynamic link and verifying it', () => { + cy.visit('/app/test-dynamic-link'); + + //Clicking on the Document ID field + cy.get_field('doc_type').clear(); + + //Entering User in the Doctype field + cy.fill_field('doc_type', 'User', 'Link', {delay: 500}); + cy.get_field('doc_id').click(); + + //Checking if the listbox have length greater than 0 + cy.get('[data-fieldname="doc_id"]').find('.awesomplete').find("li").its('length').should('be.gte', 0); + + //Opening a new form for dynamic link doctype + cy.new_form('Test Dynamic Link'); + cy.get_field('doc_type').clear(); + + //Entering User in the Doctype field + cy.fill_field('doc_type', 'User', 'Link', {delay: 500}); + cy.get_field('doc_id').click(); + + //Checking if the listbox have length greater than 0 + cy.get('[data-fieldname="doc_id"]').find('.awesomplete').find("li").its('length').should('be.gte', 0); + cy.get_field('doc_type').clear(); + + //Entering System Settings in the Doctype field + cy.fill_field('doc_type', 'System Settings', 'Link', {delay: 500}); + cy.get_field('doc_id').click(); + + //Checking if the system throws error + cy.get('.modal-title').should('have.text', 'Error'); + cy.get('.msgprint').should('have.text', 'System Settings is not a valid DocType for Dynamic Link'); + }); +}); \ No newline at end of file diff --git a/cypress/integration/web_form.js b/cypress/integration/web_form.js index 8346c96313..bd1c7e147e 100644 --- a/cypress/integration/web_form.js +++ b/cypress/integration/web_form.js @@ -7,8 +7,8 @@ context('Web Form', () => { cy.visit('/update-profile'); cy.get_field('last_name', 'Data').type('_Test User', {force: true}).wait(200); cy.get('.web-form-actions .btn-primary').click(); - cy.wait(500); - cy.get('.modal.show > .modal-dialog').should('be.visible'); + cy.wait(5000); + cy.url().should('include', '/me'); }); it('Navigate and Submit a MultiStep WebForm', () => { @@ -16,14 +16,12 @@ context('Web Form', () => { cy.visit('/update-profile-duplicate'); cy.get_field('last_name', 'Data').type('_Test User', {force: true}).wait(200); cy.get('.btn-next').should('be.visible'); - cy.get('.web-form-footer .btn-primary').should('not.be.visible'); cy.get('.btn-next').click(); cy.get('.btn-previous').should('be.visible'); cy.get('.btn-next').should('not.be.visible'); - cy.get('.web-form-footer .btn-primary').should('be.visible'); cy.get('.web-form-actions .btn-primary').click(); - cy.wait(500); - cy.get('.modal.show > .modal-dialog').should('be.visible'); + cy.wait(5000); + cy.url().should('include', '/me'); }); }); }); diff --git a/cypress/support/index.js b/cypress/support/index.js index 9cd770a31e..5980e96677 100644 --- a/cypress/support/index.js +++ b/cypress/support/index.js @@ -17,6 +17,9 @@ import './commands'; import '@cypress/code-coverage/support'; +Cypress.on('uncaught:exception', (err, runnable) => { + return false; +}); // Alternatively you can use CommonJS syntax: // require('./commands') diff --git a/frappe/__init__.py b/frappe/__init__.py index ac0d1a3b8f..0eca4d99d0 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.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 """ Frappe - Low Code Open Source Framework in Python and JS @@ -20,10 +20,10 @@ if _dev_server: warnings.simplefilter('always', DeprecationWarning) warnings.simplefilter('always', PendingDeprecationWarning) -from werkzeug.local import Local, release_local import sys, importlib, inspect, json -import typing import click +from werkzeug.local import Local, release_local +from typing import TYPE_CHECKING, Dict, List, Union # Local application imports from .exceptions import * @@ -143,15 +143,14 @@ lang = local("lang") # This if block is never executed when running the code. It is only used for # telling static code analyzer where to find dynamically defined attributes. -if typing.TYPE_CHECKING: - from frappe.utils.redis_wrapper import RedisWrapper - +if TYPE_CHECKING: from frappe.database.mariadb.database import MariaDBDatabase from frappe.database.postgres.database import PostgresDatabase from frappe.query_builder.builder import MariaDB, Postgres + from frappe.utils.redis_wrapper import RedisWrapper - db: typing.Union[MariaDBDatabase, PostgresDatabase] - qb: typing.Union[MariaDB, Postgres] + db: Union[MariaDBDatabase, PostgresDatabase] + qb: Union[MariaDB, Postgres] # end: static analysis hack @@ -897,7 +896,12 @@ def clear_document_cache(doctype, name): cache().hdel('document_cache', key) def get_cached_value(doctype, name, fieldname, as_dict=False): - doc = get_cached_doc(doctype, name) + try: + doc = get_cached_doc(doctype, name) + except DoesNotExistError: + clear_last_message() + return + if isinstance(fieldname, str): if as_dict: throw('Cannot make dict for single fieldname') @@ -1465,7 +1469,7 @@ def get_list(doctype, *args, **kwargs): :param fields: List of fields or `*`. :param filters: List of filters (see example). :param order_by: Order By e.g. `modified desc`. - :param limit_page_start: Start results at record #. Default 0. + :param limit_start: Start results at record #. Default 0. :param limit_page_length: No of records in the page. Default 20. Example usage: @@ -1523,12 +1527,16 @@ def get_value(*args, **kwargs): """ return db.get_value(*args, **kwargs) -def as_json(obj, indent=1): +def as_json(obj: Union[Dict, List], indent=1) -> str: from frappe.utils.response import json_handler + try: return json.dumps(obj, indent=indent, sort_keys=True, default=json_handler, separators=(',', ': ')) except TypeError: - return json.dumps(obj, indent=indent, default=json_handler, separators=(',', ': ')) + # this would break in case the keys are not all os "str" type - as defined in the JSON + # adding this to ensure keys are sorted (expected behaviour) + sorted_obj = dict(sorted(obj.items(), key=lambda kv: str(kv[0]))) + return json.dumps(sorted_obj, indent=indent, default=json_handler, separators=(',', ': ')) def are_emails_muted(): from frappe.utils import cint diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 63da4db093..e788c7ec4d 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -728,7 +728,7 @@ def move(dest_dir, site): @click.command('set-password') @click.argument('user') @click.argument('password', required=False) -@click.option('--logout-all-sessions', help='Logout from all sessions', is_flag=True, default=False) +@click.option('--logout-all-sessions', help='Log out from all sessions', is_flag=True, default=False) @pass_context def set_password(context, user, password=None, logout_all_sessions=False): "Set password for a user on a site" @@ -741,7 +741,7 @@ def set_password(context, user, password=None, logout_all_sessions=False): @click.command('set-admin-password') @click.argument('admin-password', required=False) -@click.option('--logout-all-sessions', help='Logout from all sessions', is_flag=True, default=False) +@click.option('--logout-all-sessions', help='Log out from all sessions', is_flag=True, default=False) @pass_context def set_admin_password(context, admin_password=None, logout_all_sessions=False): "Set Administrator password for a site" diff --git a/frappe/core/doctype/communication/communication.json b/frappe/core/doctype/communication/communication.json index 175c64b9eb..f9d15af483 100644 --- a/frappe/core/doctype/communication/communication.json +++ b/frappe/core/doctype/communication/communication.json @@ -153,7 +153,7 @@ "fieldname": "communication_type", "fieldtype": "Select", "label": "Communication Type", - "options": "Communication\nComment\nChat\nBot\nNotification\nFeedback\nAutomated Message", + "options": "Communication\nComment\nChat\nNotification\nFeedback\nAutomated Message", "read_only": 1, "reqd": 1 }, @@ -164,7 +164,7 @@ "in_list_view": 1, "in_standard_filter": 1, "label": "Comment Type", - "options": "\nComment\nLike\nInfo\nLabel\nWorkflow\nCreated\nSubmitted\nCancelled\nUpdated\nDeleted\nAssigned\nAssignment Completed\nAttachment\nAttachment Removed\nShared\nUnshared\nBot\nRelinked", + "options": "\nComment\nLike\nInfo\nLabel\nWorkflow\nCreated\nSubmitted\nCancelled\nUpdated\nDeleted\nAssigned\nAssignment Completed\nAttachment\nAttachment Removed\nShared\nUnshared\nRelinked", "read_only": 1 }, { @@ -395,7 +395,7 @@ "icon": "fa fa-comment", "idx": 1, "links": [], - "modified": "2021-11-30 09:03:25.728637", + "modified": "2022-03-30 11:24:25.728637", "modified_by": "Administrator", "module": "Core", "name": "Communication", diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 475762f39d..38fb0fd757 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -10,7 +10,6 @@ from frappe.utils import validate_email_address, strip_html, cstr, time_diff_in_ from frappe.core.doctype.communication.email import validate_email from frappe.core.doctype.communication.mixins import CommunicationEmailMixin from frappe.core.utils import get_parent_doc -from frappe.utils.bot import BotReply from frappe.utils import parse_addr, split_emails from frappe.core.doctype.comment.comment import update_comment_in_doc from email.utils import getaddresses @@ -105,7 +104,7 @@ class Communication(Document, CommunicationEmailMixin): if self.communication_type == "Communication": self.notify_change('add') - elif self.communication_type in ("Chat", "Notification", "Bot"): + elif self.communication_type in ("Chat", "Notification"): if self.reference_name == frappe.session.user: message = self.as_dict() message['broadcast'] = True @@ -160,7 +159,6 @@ class Communication(Document, CommunicationEmailMixin): if self.comment_type != 'Updated': update_parent_document_on_communication(self) - self.bot_reply() def on_trash(self): if self.communication_type == "Communication": @@ -278,20 +276,6 @@ class Communication(Document, CommunicationEmailMixin): if not self.sender_full_name: self.sender_full_name = sender_email - def bot_reply(self): - if self.comment_type == 'Bot' and self.communication_type == 'Chat': - reply = BotReply().get_reply(self.content) - if reply: - frappe.get_doc({ - "doctype": "Communication", - "comment_type": "Bot", - "communication_type": "Bot", - "content": cstr(reply), - "reference_doctype": self.reference_doctype, - "reference_name": self.reference_name - }).insert() - frappe.local.flags.commit = True - def set_delivery_status(self, commit=False): '''Look into the status of Email Queue linked to this Communication and set the Delivery Status of this Communication''' delivery_status = None diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index 7c9c7b5067..f398577665 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -2,20 +2,17 @@ # License: MIT. See LICENSE from datetime import datetime -import unittest import frappe from frappe.utils import now_datetime, add_to_date from frappe.core.doctype.log_settings.log_settings import run_log_clean_up +from frappe.tests.utils import FrappeTestCase -class TestLogSettings(unittest.TestCase): +class TestLogSettings(FrappeTestCase): @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) + super().setUpClass() frappe.db.set_single_value( "Log Settings", @@ -26,10 +23,6 @@ class TestLogSettings(unittest.TestCase): }, ) - @classmethod - def tearDownClass(cls): - frappe.db.rollback(save_point=cls.savepoint) - def setUp(self) -> None: if self._testMethodName == "test_delete_logs": self.datetime = frappe._dict() diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 9cb40dffd4..bf82a3f684 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -11,7 +11,7 @@ from frappe.modules import make_boilerplate from frappe.core.doctype.page.page import delete_custom_role from frappe.core.doctype.custom_role.custom_role import get_custom_allowed_roles from frappe.desk.reportview import append_totals_row -from frappe.utils.safe_exec import safe_exec +from frappe.utils.safe_exec import safe_exec, check_safe_sql_query class Report(Document): @@ -110,8 +110,7 @@ class Report(Document): if not self.query: frappe.throw(_("Must specify a Query to run"), title=_('Report Document Error')) - if not self.query.lower().startswith("select"): - frappe.throw(_("Query must be a SELECT"), title=_('Report Document Error')) + check_safe_sql_query(self.query) result = [list(t) for t in frappe.db.sql(self.query, filters)] columns = self.get_columns() or [cstr(c[0]) for c in frappe.db.get_description()] diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index a077956d71..e58d038993 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -1,17 +1,19 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import textwrap + import frappe, json, os -import unittest from frappe.desk.query_report import run, save_report, add_total_row from frappe.desk.reportview import delete_report, save_report as _save_report from frappe.custom.doctype.customize_form.customize_form import reset_customization from frappe.core.doctype.user_permission.test_user_permission import create_user +from frappe.tests.utils import FrappeTestCase test_records = frappe.get_test_records('Report') test_dependencies = ['User'] -class TestReport(unittest.TestCase): +class TestReport(FrappeTestCase): def test_report_builder(self): if frappe.db.exists('Report', 'User Activity Report'): frappe.delete_doc('Report', 'User Activity Report') @@ -335,3 +337,29 @@ result = [ self.assertEqual(result[-1][0], "Total") self.assertEqual(result[-1][1], 200) self.assertEqual(result[-1][2], 150.50) + + def test_cte_in_query_report(self): + cte_query = textwrap.dedent(""" + with enabled_users as ( + select name + from `tabUser` + where enabled = 1 + ) + select * from enabled_users; + """) + + report = frappe.get_doc({ + "doctype": "Report", + "ref_doctype": "User", + "report_name": "Enabled Users List", + "report_type": "Query Report", + "is_standard": "No", + "query": cte_query, + }).insert() + + if frappe.db.db_type == "mariadb": + col, rows = report.execute_query_report(filters={}) + self.assertEqual(col[0], "name") + self.assertGreaterEqual(len(rows), 1) + elif frappe.db.db_type == "postgres": + self.assertRaises(frappe.PermissionError, report.execute_query_report, filters={}) diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index 9e9529cd5e..642a392a58 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -2,7 +2,7 @@ "actions": [], "allow_import": 1, "allow_rename": 1, - "creation": "2014-03-11 14:55:00", + "creation": "2022-01-10 17:29:51.672911", "description": "Represents a User in the system.", "doctype": "DocType", "engine": "InnoDB", @@ -48,6 +48,12 @@ "document_follow_notifications_section", "document_follow_notify", "document_follow_frequency", + "column_break_75", + "follow_created_documents", + "follow_commented_documents", + "follow_liked_documents", + "follow_assigned_documents", + "follow_shared_documents", "email_settings", "email_signature", "thread_notify", @@ -606,6 +612,45 @@ "fieldtype": "Link", "label": "Module Profile", "options": "Module Profile" + }, + { + "fieldname": "column_break_75", + "fieldtype": "Column Break" + }, + { + "default": "0", + "depends_on": "eval:(doc.document_follow_notify== 1)", + "fieldname": "follow_created_documents", + "fieldtype": "Check", + "label": "Auto follow documents that you create" + }, + { + "default": "0", + "depends_on": "eval:(doc.document_follow_notify== 1)", + "fieldname": "follow_commented_documents", + "fieldtype": "Check", + "label": "Auto follow documents that you comment on" + }, + { + "default": "0", + "depends_on": "eval:(doc.document_follow_notify== 1)", + "fieldname": "follow_liked_documents", + "fieldtype": "Check", + "label": "Auto follow documents that you Like" + }, + { + "default": "0", + "depends_on": "eval:(doc.document_follow_notify== 1)", + "fieldname": "follow_shared_documents", + "fieldtype": "Check", + "label": "Auto follow documents that are shared with you" + }, + { + "default": "0", + "depends_on": "eval:(doc.document_follow_notify== 1)", + "fieldname": "follow_assigned_documents", + "fieldtype": "Check", + "label": "Auto follow documents that are assigned to you" } ], "icon": "fa fa-user", @@ -704,4 +749,4 @@ "states": [], "title_field": "full_name", "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/core/web_form/edit_profile/edit_profile.json b/frappe/core/web_form/edit_profile/edit_profile.json index 7072584670..c04e705820 100644 --- a/frappe/core/web_form/edit_profile/edit_profile.json +++ b/frappe/core/web_form/edit_profile/edit_profile.json @@ -1,133 +1,159 @@ { - "accept_payment": 0, - "allow_comments": 0, - "allow_delete": 0, - "allow_edit": 1, - "allow_incomplete": 0, - "allow_multiple": 0, - "allow_print": 0, - "amount": 0.0, - "amount_based_on_field": 0, - "breadcrumbs": "[{\"title\": _(\"My Account\"), \"route\": \"me\"}]", - "creation": "2016-09-19 05:16:59.242754", - "doc_type": "User", - "docstatus": 0, - "doctype": "Web Form", - "idx": 0, - "introduction_text": "", - "is_standard": 1, - "login_required": 1, - "max_attachment_size": 0, - "modified": "2019-01-28 12:45:17.158069", - "modified_by": "Administrator", - "module": "Core", - "name": "edit-profile", - "owner": "Administrator", - "published": 1, - "route": "update-profile", - "show_in_grid": 0, - "show_sidebar": 1, - "sidebar_items": [], - "success_message": "Profile updated successfully.", - "success_url": "/me", - "title": "Update Profile", + "accept_payment": 0, + "allow_comments": 0, + "allow_delete": 0, + "allow_edit": 1, + "allow_incomplete": 0, + "allow_multiple": 0, + "allow_print": 0, + "amount": 0.0, + "amount_based_on_field": 0, + "apply_document_permissions": 0, + "breadcrumbs": "[{\"title\": _(\"My Account\"), \"route\": \"me\"}]", + "creation": "2016-09-19 05:16:59.242754", + "doc_type": "User", + "docstatus": 0, + "doctype": "Web Form", + "idx": 0, + "introduction_text": "", + "is_multi_step_form": 0, + "is_standard": 1, + "login_required": 1, + "max_attachment_size": 0, + "modified": "2022-03-22 15:00:43.456738", + "modified_by": "Administrator", + "module": "Core", + "name": "edit-profile", + "owner": "Administrator", + "published": 1, + "route": "update-profile", + "route_to_success_link": 0, + "show_attachments": 0, + "show_in_grid": 0, + "show_sidebar": 0, + "sidebar_items": [], + "success_message": "Profile updated successfully.", + "success_url": "/me", + "title": "Update Profile", "web_form_fields": [ { - "allow_read_on_all_link_options": 0, - "fieldname": "first_name", - "fieldtype": "Data", - "hidden": 0, - "label": "First Name", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 1, + "allow_read_on_all_link_options": 0, + "fieldname": "first_name", + "fieldtype": "Data", + "hidden": 0, + "label": "First Name", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 1, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "fieldname": "middle_name", - "fieldtype": "Data", - "hidden": 0, - "label": "Middle Name (Optional)", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldname": "middle_name", + "fieldtype": "Data", + "hidden": 0, + "label": "Middle Name (Optional)", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "fieldname": "last_name", - "fieldtype": "Data", - "hidden": 0, - "label": "Last Name", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldname": "last_name", + "fieldtype": "Data", + "hidden": 0, + "label": "Last Name", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "description": "", - "fieldname": "user_image", - "fieldtype": "Attach Image", - "hidden": 0, - "label": "User Image", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldname": "", + "fieldtype": "Column Break", + "hidden": 0, + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "fieldtype": "Section Break", - "hidden": 0, - "label": "More Information", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "description": "", + "fieldname": "user_image", + "fieldtype": "Attach Image", + "hidden": 0, + "label": "Profile Picture", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "fieldname": "phone", - "fieldtype": "Data", - "hidden": 0, - "label": "Phone", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldtype": "Section Break", + "hidden": 0, + "label": "More Information", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "fieldname": "mobile_no", - "fieldtype": "Data", - "hidden": 0, - "label": "Mobile Number", - "max_length": 0, - "max_value": 0, - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldname": "phone", + "fieldtype": "Data", + "hidden": 0, + "label": "Phone", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, "show_in_filter": 0 - }, + }, { - "allow_read_on_all_link_options": 0, - "description": "", - "fieldname": "language", - "fieldtype": "Link", - "hidden": 0, - "label": "Language", - "max_length": 0, - "max_value": 0, - "options": "Language", - "read_only": 0, - "reqd": 0, + "allow_read_on_all_link_options": 0, + "fieldname": "mobile_no", + "fieldtype": "Data", + "hidden": 0, + "label": "Mobile Number", + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, + "show_in_filter": 0 + }, + { + "allow_read_on_all_link_options": 0, + "fieldname": "", + "fieldtype": "Column Break", + "hidden": 0, + "max_length": 0, + "max_value": 0, + "read_only": 0, + "reqd": 0, + "show_in_filter": 0 + }, + { + "allow_read_on_all_link_options": 0, + "description": "", + "fieldname": "language", + "fieldtype": "Link", + "hidden": 0, + "label": "Language", + "max_length": 0, + "max_value": 0, + "options": "Language", + "read_only": 0, + "reqd": 0, "show_in_filter": 0 } ] diff --git a/frappe/database/database.py b/frappe/database/database.py index 24dfdd32df..511d993aa5 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -119,8 +119,8 @@ class Database(object): if not run: return query - # remove \n \t from start and end of query - query = re.sub(r'^\s*|\s*$', '', query) + # remove whitespace / indentation from start and end of query + query = query.strip() if re.search(r'ifnull\(', query, flags=re.IGNORECASE): # replaces ifnull in query with coalesce @@ -357,6 +357,7 @@ class Database(object): order_by="KEEP_DEFAULT_ORDERING", cache=False, for_update=False, + *, run=True, pluck=False, distinct=False, @@ -386,17 +387,27 @@ class Database(object): frappe.db.get_value("System Settings", None, "date_format") """ - ret = self.get_values(doctype, filters, fieldname, ignore, as_dict, debug, + result = self.get_values(doctype, filters, fieldname, ignore, as_dict, debug, order_by, cache=cache, for_update=for_update, run=run, pluck=pluck, distinct=distinct, limit=1) if not run: - return ret + return result + + if not result: + return None + + row = result[0] + + if len(row) > 1 or as_dict: + return row + else: + # single field is requested, send it without wrapping in containers + return row[0] - return ((len(ret[0]) > 1 or as_dict) and ret[0] or ret[0][0]) if ret else None def get_values(self, doctype, filters=None, fieldname="name", ignore=None, as_dict=False, debug=False, order_by="KEEP_DEFAULT_ORDERING", update=None, cache=False, for_update=False, - run=True, pluck=False, distinct=False, limit=None): + *, run=True, pluck=False, distinct=False, limit=None): """Returns multiple document properties. :param doctype: DocType name. @@ -487,6 +498,7 @@ class Database(object): as_dict=False, debug=False, update=None, + *, run=True, pluck=False, distinct=False, @@ -621,7 +633,8 @@ class Database(object): filters, doctype, as_dict, - debug, + *, + debug=False, order_by=None, update=None, for_update=False, @@ -661,7 +674,7 @@ class Database(object): ) return r - def _get_value_for_many_names(self, doctype, names, field, order_by, debug=False, run=True, pluck=False, distinct=False, limit=None): + def _get_value_for_many_names(self, doctype, names, field, order_by, *, debug=False, run=True, pluck=False, distinct=False, limit=None): names = list(filter(None, names)) if names: return self.get_all( diff --git a/frappe/desk/form/assign_to.py b/frappe/desk/form/assign_to.py index 049d33c1ec..d79927a506 100644 --- a/frappe/desk/form/assign_to.py +++ b/frappe/desk/form/assign_to.py @@ -84,7 +84,8 @@ def add(args=None): shared_with_users.append(assign_to) # make this document followed by assigned user - follow_document(args['doctype'], args['name'], assign_to) + if frappe.get_cached_value("User", assign_to, "follow_assigned_documents"): + follow_document(args['doctype'], args['name'], assign_to) # notify notify_assignment(d.assigned_by, d.allocated_to, d.reference_type, d.reference_name, action='ASSIGN', diff --git a/frappe/desk/form/document_follow.py b/frappe/desk/form/document_follow.py index 14970092d0..7dd2b64c21 100644 --- a/frappe/desk/form/document_follow.py +++ b/frappe/desk/form/document_follow.py @@ -6,7 +6,7 @@ import frappe.utils from frappe.utils import get_url_to_form from frappe.model import log_types from frappe import _ -from itertools import groupby +from frappe.query_builder import DocType @frappe.whitelist() def update_follow(doctype, doc_name, following): @@ -94,33 +94,50 @@ def send_document_follow_mails(frequency): call method to send mail ''' - users = frappe.get_list("Document Follow", - fields=["*"]) - - sorted_users = sorted(users, key=lambda k: k['user']) - - grouped_by_user = {} - for k, v in groupby(sorted_users, key=lambda k: k['user']): - grouped_by_user[k] = list(v) - - for user in grouped_by_user: - user_frequency = frappe.db.get_value("User", user, "document_follow_frequency") - message = [] - valid_document_follows = [] - if user_frequency == frequency: - for d in grouped_by_user[user]: - content = get_message(d.ref_docname, d.ref_doctype, frequency, user) - if content: - message = message + content - valid_document_follows.append({ - "reference_docname": d.ref_docname, - "reference_doctype": d.ref_doctype, - "reference_url": get_url_to_form(d.ref_doctype, d.ref_docname) - }) - - if message and frappe.db.get_value("User", user, "document_follow_notify", ignore=True): - send_email_alert(user, valid_document_follows, message) - + user_list = get_user_list(frequency) + + for user in user_list: + message, valid_document_follows = get_message_for_user(frequency, user) + if message: + send_email_alert(user, valid_document_follows, message) + # send an email if we have already spent resources creating the message + # nosemgrep + frappe.db.commit() + +def get_user_list(frequency): + DocumentFollow = DocType('Document Follow') + User = DocType('User') + return (frappe.qb.from_(DocumentFollow).join(User) + .on(DocumentFollow.user == User.name) + .where(User.document_follow_notify == 1) + .where(User.document_follow_frequency == frequency) + .select(DocumentFollow.user) + .groupby(DocumentFollow.user)).run(pluck="user") + +def get_message_for_user(frequency, user): + message = [] + latest_document_follows = get_document_followed_by_user(user) + valid_document_follows = [] + + for document_follow in latest_document_follows: + content = get_message(document_follow.ref_docname, document_follow.ref_doctype, frequency, user) + if content: + message = message + content + valid_document_follows.append({ + "reference_docname": document_follow.ref_docname, + "reference_doctype": document_follow.ref_doctype, + "reference_url": get_url_to_form(document_follow.ref_doctype, document_follow.ref_docname) + }) + return message, valid_document_follows + +def get_document_followed_by_user(user): + DocumentFollow = DocType('Document Follow') + # at max 20 documents are sent for each user + return (frappe.qb.from_(DocumentFollow) + .where(DocumentFollow.user == user) + .select(DocumentFollow.ref_doctype, DocumentFollow.ref_docname) + .orderby(DocumentFollow.modified) + .limit(20)).run(as_dict=True) def get_version(doctype, doc_name, frequency, user): timeline = [] diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index 291767de10..f80b89d2b8 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -31,8 +31,8 @@ def add_comment(reference_doctype, reference_name, content, comment_email, comme reference_doc = frappe.get_doc(reference_doctype, reference_name) doc.content = extract_images_from_html(reference_doc, content, is_private=True) doc.insert(ignore_permissions=True) - - follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user) + if frappe.get_cached_value("User", frappe.session.user, "follow_commented_documents"): + follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user) return doc.as_dict() @frappe.whitelist() diff --git a/frappe/desk/like.py b/frappe/desk/like.py index 4480ed8a1e..5e5a789973 100644 --- a/frappe/desk/like.py +++ b/frappe/desk/like.py @@ -41,7 +41,8 @@ def _toggle_like(doctype, name, add, user=None): if user not in liked_by: liked_by.append(user) add_comment(doctype, name) - follow_document(doctype, name, user) + if frappe.get_cached_value("User", user, "follow_liked_documents"): + follow_document(doctype, name, user) else: if user in liked_by: liked_by.remove(user) diff --git a/frappe/email/doctype/document_follow/test_document_follow.py b/frappe/email/doctype/document_follow/test_document_follow.py index 050add65e9..0f6ff6c114 100644 --- a/frappe/email/doctype/document_follow/test_document_follow.py +++ b/frappe/email/doctype/document_follow/test_document_follow.py @@ -3,10 +3,18 @@ # License: MIT. See LICENSE import frappe import unittest +from dataclasses import dataclass import frappe.desk.form.document_follow as document_follow +from frappe.query_builder import DocType +from frappe.desk.form.utils import add_comment +from frappe.desk.form.document_follow import get_document_followed_by_user +from frappe.desk.like import toggle_like +from frappe.desk.form.assign_to import add +from frappe.share import add as share +from frappe.query_builder.functions import Cast_ class TestDocumentFollow(unittest.TestCase): - def test_document_follow(self): + def test_document_follow_version(self): user = get_user() event_doc = get_event() @@ -18,18 +26,173 @@ class TestDocumentFollow(unittest.TestCase): self.assertEqual(doc.user, user.name) document_follow.send_hourly_updates() + emails = get_emails(event_doc, '%This is a test description for sending mail%') + self.assertIsNotNone(emails) - email_queue_entry_name = frappe.get_all("Email Queue", limit=1)[0].name - email_queue_entry_doc = frappe.get_doc("Email Queue", email_queue_entry_name) - self.assertEqual((email_queue_entry_doc.recipients[0].recipient), user.name) + def test_document_follow_comment(self): + user = get_user() + event_doc = get_event() + + add_comment(event_doc.doctype, event_doc.name, "This is a test comment", 'Administrator@example.com', 'Bosh') + + document_follow.unfollow_document("Event", event_doc.name, user.name) + doc = document_follow.follow_document("Event", event_doc.name, user.name) + self.assertEqual(doc.user, user.name) + + document_follow.send_hourly_updates() + emails = get_emails(event_doc, '%This is a test comment%') + self.assertIsNotNone(emails) + + + def test_follow_limit(self): + user = get_user() + for _ in range(25): + event_doc = get_event() + document_follow.unfollow_document("Event", event_doc.name, user.name) + doc = document_follow.follow_document("Event", event_doc.name, user.name) + self.assertEqual(doc.user, user.name) + self.assertEqual(len(get_document_followed_by_user(user.name)), 20) + + def test_follow_on_create(self): + user = get_user(DocumentFollowConditions(1)) + frappe.set_user(user.name) + event = get_event() + + event.description = "This is a test description for sending mail" + event.save(ignore_version=False) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertTrue(documents_followed) + + def test_do_not_follow_on_create(self): + user = get_user() + frappe.set_user(user.name) + + event = get_event() + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) + + def test_do_not_follow_on_update(self): + user = get_user() + frappe.set_user(user.name) + event = get_event() + + event.description = "This is a test description for sending mail" + event.save(ignore_version=False) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) + + def test_follow_on_comment(self): + user = get_user(DocumentFollowConditions(0, 1)) + frappe.set_user(user.name) + event = get_event() + + add_comment(event.doctype, event.name, "This is a test comment", 'Administrator@example.com', 'Bosh') + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertTrue(documents_followed) + + def test_do_not_follow_on_comment(self): + user = get_user() + frappe.set_user(user.name) + event = get_event() - self.assertIn(event_doc.doctype, email_queue_entry_doc.message) - self.assertIn(event_doc.name, email_queue_entry_doc.message) + add_comment(event.doctype, event.name, "This is a test comment", 'Administrator@example.com', 'Bosh') + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) + + def test_follow_on_like(self): + user = get_user(DocumentFollowConditions(0, 0, 1)) + frappe.set_user(user.name) + event = get_event() + + toggle_like(event.doctype, event.name, add="Yes") + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertTrue(documents_followed) + + def test_do_not_follow_on_like(self): + user = get_user() + frappe.set_user(user.name) + event = get_event() + + toggle_like(event.doctype, event.name) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) + + def test_follow_on_assign(self): + user = get_user(DocumentFollowConditions(0, 0, 0, 1)) + event = get_event() + + add({ + 'assign_to': [user.name], + 'doctype': event.doctype, + 'name': event.name + }) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertTrue(documents_followed) + + def test_do_not_follow_on_assign(self): + user = get_user() + frappe.set_user(user.name) + event = get_event() + + add({ + 'assign_to': [user.name], + 'doctype': event.doctype, + 'name': event.name + }) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) + + def test_follow_on_share(self): + user = get_user(DocumentFollowConditions(0, 0, 0, 0, 1)) + event = get_event() + + share( + user= user.name, + doctype= event.doctype, + name= event.name + ) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertTrue(documents_followed) + + def test_do_not_follow_on_share(self): + user = get_user() + event = get_event() + + share( + user = user.name, + doctype = event.doctype, + name = event.name + ) + + documents_followed = get_events_followed_by_user(event.name, user.name) + self.assertFalse(documents_followed) def tearDown(self): frappe.db.rollback() + frappe.db.delete('Email Queue') + frappe.db.delete('Email Queue Recipient') + frappe.db.delete('Document Follow') + frappe.db.delete('Event') + +def get_events_followed_by_user(event_name, user_name): + DocumentFollow = DocType('Document Follow') + return (frappe.qb.from_(DocumentFollow) + .where(DocumentFollow.ref_doctype == 'Event') + .where(DocumentFollow.ref_docname == event_name) + .where(DocumentFollow.user == user_name) + .select(DocumentFollow.name)).run() def get_event(): doc = frappe.get_doc({ @@ -42,16 +205,40 @@ def get_event(): doc.insert() return doc -def get_user(): +def get_user(document_follow=None): + frappe.set_user("Administrator") if frappe.db.exists('User', 'test@docsub.com'): - doc = frappe.get_doc('User', 'test@docsub.com') - else: - doc = frappe.new_doc("User") - doc.email = "test@docsub.com" - doc.first_name = "Test" - doc.last_name = "User" - doc.send_welcome_email = 0 - doc.document_follow_notify = 1 - doc.document_follow_frequency = "Hourly" - doc.insert() - return doc \ No newline at end of file + doc = frappe.delete_doc('User', 'test@docsub.com') + doc = frappe.new_doc("User") + doc.email = "test@docsub.com" + doc.first_name = "Test" + doc.last_name = "User" + doc.send_welcome_email = 0 + doc.document_follow_notify = 1 + doc.document_follow_frequency = "Hourly" + doc.__dict__.update(document_follow.__dict__ if document_follow else {}) + doc.insert() + doc.add_roles('System Manager') + return doc + +def get_emails(event_doc, search_string): + EmailQueue = DocType('Email Queue') + EmailQueueRecipient = DocType('Email Queue Recipient') + + return (frappe.qb.from_(EmailQueue) + .join(EmailQueueRecipient) + .on(EmailQueueRecipient.parent == Cast_(EmailQueue.name, "varchar")) + .where(EmailQueueRecipient.recipient == 'test@docsub.com',) + .where(EmailQueue.message.like(f'%{event_doc.doctype}%')) + .where(EmailQueue.message.like(f'%{event_doc.name}%')) + .where(EmailQueue.message.like(search_string)) + .select(EmailQueue.message) + .limit(1)).run() + +@dataclass +class DocumentFollowConditions: + follow_created_documents: int = 0 + follow_commented_documents: int = 0 + follow_liked_documents: int = 0 + follow_assigned_documents: int = 0 + follow_shared_documents: int = 0 diff --git a/frappe/hooks.py b/frappe/hooks.py index 78f4a2d801..b545c6a719 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -282,14 +282,6 @@ sounds = [ # {"name": "chime", "src": "/assets/frappe/sounds/chime.mp3"}, ] -bot_parsers = [ - 'frappe.utils.bot.ShowNotificationBot', - 'frappe.utils.bot.GetOpenListBot', - 'frappe.utils.bot.ListBot', - 'frappe.utils.bot.FindBot', - 'frappe.utils.bot.CountBot' -] - setup_wizard_exception = [ "frappe.desk.page.setup_wizard.setup_wizard.email_setup_wizard_exception", "frappe.desk.page.setup_wizard.setup_wizard.log_setup_wizard_exception" diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 57b4777355..8fd64689fc 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -963,10 +963,13 @@ class BaseDocument(object): from frappe.model.meta import get_default_df df = get_default_df(fieldname) - if df.fieldtype == "Currency" and not currency: - currency = self.get(df.get("options")) - if not frappe.db.exists('Currency', currency, cache=True): - currency = None + if ( + df.fieldtype == "Currency" + and not currency + and (currency_field := df.get("options")) + and (currency_value := self.get(currency_field)) + ): + currency = frappe.db.get_value('Currency', currency_value, cache=True) val = self.get(fieldname) diff --git a/frappe/model/document.py b/frappe/model/document.py index 3c38ff3442..3848fa8029 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -276,7 +276,8 @@ class Document(BaseDocument): delattr(self, "__unsaved") if not (frappe.flags.in_migrate or frappe.local.flags.in_install or frappe.flags.in_setup_wizard): - follow_document(self.doctype, self.name, frappe.session.user) + if frappe.get_cached_value("User", frappe.session.user, "follow_created_documents"): + follow_document(self.doctype, self.name, frappe.session.user) return self def save(self, *args, **kwargs): @@ -1125,7 +1126,8 @@ class Document(BaseDocument): version.insert(ignore_permissions=True) if not frappe.flags.in_migrate: # follow since you made a change? - follow_document(self.doctype, self.name, frappe.session.user) + if frappe.get_cached_value("User", frappe.session.user, "follow_created_documents"): + follow_document(self.doctype, self.name, frappe.session.user) @staticmethod def hook(f): diff --git a/frappe/modules/utils.py b/frappe/modules/utils.py index 13b52d2020..4768faff48 100644 --- a/frappe/modules/utils.py +++ b/frappe/modules/utils.py @@ -272,7 +272,7 @@ def make_boilerplate(template, doc, opts=None): frappe.utils.cstr(source.read()).format( app_publisher=app_publisher, year=frappe.utils.nowdate()[:4], - classname=doc.name.replace(" ", ""), + classname=doc.name.replace(" ", "").replace("-", ""), base_class_import=base_class_import, base_class=base_class, doctype=doc.name, **opts, diff --git a/frappe/patches.txt b/frappe/patches.txt index e85c837286..bc2bc22637 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -146,7 +146,7 @@ frappe.patches.v13_0.update_duration_options frappe.patches.v13_0.replace_old_data_import # 2020-06-24 frappe.patches.v13_0.create_custom_dashboards_cards_and_charts frappe.patches.v13_0.rename_is_custom_field_in_dashboard_chart -frappe.patches.v13_0.add_standard_navbar_items # 2022-03-15 +frappe.patches.v13_0.add_standard_navbar_items # 2020-12-15 frappe.patches.v13_0.generate_theme_files_in_public_folder frappe.patches.v13_0.increase_password_length frappe.patches.v12_0.fix_email_id_formatting diff --git a/frappe/printing/page/print/print.js b/frappe/printing/page/print/print.js index 5d04fbe982..267419a887 100644 --- a/frappe/printing/page/print/print.js +++ b/frappe/printing/page/print/print.js @@ -37,7 +37,7 @@ frappe.ui.form.PrintView = class { this.print_wrapper = this.page.main.empty().html( `
{{ _("Comments") }}
- {% include 'templates/includes/comments/comments.html' %} -