diff --git a/cypress/integration/form.js b/cypress/integration/form.js index 4d50a5f66a..b395ff77b2 100644 --- a/cypress/integration/form.js +++ b/cypress/integration/form.js @@ -17,7 +17,8 @@ context('Form', () => { cy.get('.primary-action').click(); cy.wait('@form_save').its('response.statusCode').should('eq', 200); - cy.visit('/app/todo'); + cy.go_to_list('ToDo'); + cy.clear_filters() cy.get('.page-head').findByTitle('To Do').should('exist'); cy.get('.list-row').should('contain', 'this is a test todo'); }); diff --git a/cypress/integration/list_paging.js b/cypress/integration/list_paging.js index 4a59024a7b..0cf6f2e565 100644 --- a/cypress/integration/list_paging.js +++ b/cypress/integration/list_paging.js @@ -9,6 +9,7 @@ context('List Paging', () => { it('test load more with count selection buttons', () => { cy.visit('/app/todo/view/report'); + cy.clear_filters() cy.get('.list-paging-area .list-count').should('contain.text', '20 of'); cy.get('.list-paging-area .btn-more').click(); diff --git a/cypress/integration/list_view.js b/cypress/integration/list_view.js index 3e0d1c9d50..ee12b37638 100644 --- a/cypress/integration/list_view.js +++ b/cypress/integration/list_view.js @@ -9,6 +9,7 @@ context('List View', () => { it('Keep checkbox checked after Refresh', () => { cy.go_to_list('ToDo'); + cy.clear_filters() cy.get('.list-row-container .list-row-checkbox').click({ multiple: true, force: true }); cy.get('.actions-btn-group button').contains('Actions').should('be.visible'); cy.intercept('/api/method/frappe.desk.reportview.get').as('list-refresh'); @@ -21,6 +22,7 @@ context('List View', () => { it('enables "Actions" button', () => { const actions = ['Approve', 'Reject', 'Edit', 'Export', 'Assign To', 'Apply Assignment Rule', 'Add Tags', 'Print', 'Delete']; cy.go_to_list('ToDo'); + cy.clear_filters() cy.get('.list-row-container:contains("Pending") .list-row-checkbox').click({ multiple: true, force: true }); cy.get('.actions-btn-group button').contains('Actions').should('be.visible').click(); cy.get('.dropdown-menu li:visible .dropdown-item').should('have.length', 9).each((el, index) => { diff --git a/cypress/integration/timeline.js b/cypress/integration/timeline.js index cb4d43a96a..e7308fbaa7 100644 --- a/cypress/integration/timeline.js +++ b/cypress/integration/timeline.js @@ -12,7 +12,8 @@ context('Timeline', () => { cy.get('[data-fieldname="description"] .ql-editor.ql-blank').type('Test ToDo', {force: true}).wait(200); cy.get('.page-head .page-actions').findByRole('button', {name: 'Save'}).click(); - cy.visit('/app/todo'); + cy.go_to_list('ToDo'); + cy.clear_filters() cy.click_listview_row_item(0); //To check if the comment box is initially empty and tying some text into it @@ -79,4 +80,4 @@ context('Timeline', () => { cy.get('.page-actions .actions-btn-group [data-label="Delete"]').click(); cy.click_modal_primary_button('Yes'); }); -}); \ No newline at end of file +}); diff --git a/frappe/__init__.py b/frappe/__init__.py index 8833242a6f..3057eacd3b 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -22,7 +22,12 @@ from typing import TYPE_CHECKING, Any, Callable, Literal, Optional, overload import click from werkzeug.local import Local, release_local -from frappe.query_builder import get_query_builder, patch_query_aggregation, patch_query_execute +from frappe.query_builder import ( + get_qb_engine, + get_query_builder, + patch_query_aggregation, + patch_query_execute, +) from frappe.utils.caching import request_cache from frappe.utils.data import cstr, sbool @@ -238,7 +243,7 @@ def init(site: str, sites_path: str = ".", new_site: bool = False) -> None: local.session = _dict() local.dev_server = _dev_server local.qb = get_query_builder(local.conf.db_type or "mariadb") - + local.qb.engine = get_qb_engine() setup_module_map() if not _qb_patched.get(local.conf.db_type): diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 1b63914030..585478c0ff 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -872,7 +872,6 @@ def run_ui_tests( and os.path.exists(real_events_plugin_path) and os.path.exists(testing_library_path) and os.path.exists(coverage_plugin_path) - and cint(subprocess.getoutput("npm view cypress version")[:1]) >= 6 ): # install cypress click.secho("Installing Cypress...", fg="yellow") diff --git a/frappe/core/doctype/communication/mixins.py b/frappe/core/doctype/communication/mixins.py index 695b8bebae..ad74b47026 100644 --- a/frappe/core/doctype/communication/mixins.py +++ b/frappe/core/doctype/communication/mixins.py @@ -92,7 +92,7 @@ class CommunicationEmailMixin: cc_list = self.mail_cc( is_inbound_mail_communcation=is_inbound_mail_communcation, include_sender=include_sender ) - return [self.get_email_with_displayname(email) for email in cc_list] + return [self.get_email_with_displayname(email) for email in cc_list if email] def mail_bcc(self, is_inbound_mail_communcation=False): """ @@ -120,7 +120,7 @@ class CommunicationEmailMixin: def get_mail_bcc_with_displayname(self, is_inbound_mail_communcation=False): bcc_list = self.mail_bcc(is_inbound_mail_communcation=is_inbound_mail_communcation) - return [self.get_email_with_displayname(email) for email in bcc_list] + return [self.get_email_with_displayname(email) for email in bcc_list if email] def mail_sender(self): email_account = self.get_outgoing_email_account() diff --git a/frappe/core/doctype/user/user.js b/frappe/core/doctype/user/user.js index 001aae4da0..41b7e7fb38 100644 --- a/frappe/core/doctype/user/user.js +++ b/frappe/core/doctype/user/user.js @@ -173,14 +173,16 @@ frappe.ui.form.on('User', { }); } - frm.add_custom_button(__("Reset OTP Secret"), function() { - frappe.call({ - method: "frappe.twofactor.reset_otp_secret", - args: { - "user": frm.doc.name - } - }); - }, __("Password")); + if (frappe.session.user == doc.name || frappe.user.has_role("System Manager")) { + frm.add_custom_button(__("Reset OTP Secret"), function() { + frappe.call({ + method: "frappe.twofactor.reset_otp_secret", + args: { + "user": frm.doc.name + } + }); + }, __("Password")); + } frm.trigger('enabled'); diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index bc5c20eb92..12a48afe7e 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -134,11 +134,11 @@ class User(Document): if self.time_zone: frappe.defaults.set_default("time_zone", self.time_zone, self.name) - if self.has_value_changed("allow_in_mentions") or self.has_value_changed("user_type"): - frappe.cache().delete_key("users_for_mentions") - if self.has_value_changed("enabled"): + frappe.cache().delete_key("users_for_mentions") frappe.cache().delete_key("enabled_users") + elif self.has_value_changed("allow_in_mentions") or self.has_value_changed("user_type"): + frappe.cache().delete_key("users_for_mentions") def has_website_permission(self, ptype, user, verbose=False): """Returns true if current user is the session user""" @@ -763,19 +763,11 @@ def has_email_account(email): @frappe.whitelist(allow_guest=False) def get_email_awaiting(user): - waiting = frappe.get_all( + return frappe.get_all( "User Email", fields=["email_account", "email_id"], - filters={"awaiting_password": 1, "parent": user}, + filters={"awaiting_password": 1, "parent": user, "used_oauth": 0}, ) - if waiting: - return waiting - else: - user_email_table = DocType("User Email") - frappe.qb.update(user_email_table).set(user_email_table.user_email_table, 0).where( - user_email_table.parent == user - ).run() - return False def ask_pass_update(): @@ -783,7 +775,7 @@ def ask_pass_update(): from frappe.utils import set_default password_list = frappe.get_all( - "User Email", filters={"awaiting_password": True}, pluck="parent", distinct=True + "User Email", filters={"awaiting_password": 1, "used_oauth": 0}, pluck="parent", distinct=True ) set_default("email_user_password", ",".join(password_list)) diff --git a/frappe/core/doctype/user_email/user_email.json b/frappe/core/doctype/user_email/user_email.json index b106ed4a19..6e3f813035 100644 --- a/frappe/core/doctype/user_email/user_email.json +++ b/frappe/core/doctype/user_email/user_email.json @@ -9,6 +9,7 @@ "email_id", "column_break_3", "awaiting_password", + "used_oauth", "enable_outgoing" ], "fields": [ @@ -48,16 +49,25 @@ "fieldtype": "Check", "label": "Enable Outgoing", "read_only": 1 + }, + { + "default": "0", + "fieldname": "used_oauth", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Used OAuth", + "read_only": 1 } ], "istable": 1, "links": [], - "modified": "2020-04-06 19:19:12.130246", + "modified": "2022-06-03 14:25:46.944733", "modified_by": "Administrator", "module": "Core", "name": "User Email", "owner": "Administrator", "permissions": [], "sort_field": "modified", - "sort_order": "DESC" + "sort_order": "DESC", + "states": [] } \ No newline at end of file diff --git a/frappe/database/database.py b/frappe/database/database.py index 490b923678..a7906bc3fb 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -11,7 +11,7 @@ import string from contextlib import contextmanager from time import time -from pypika.terms import Criterion, NullValue, PseudoColumn +from pypika.terms import Criterion, NullValue import frappe import frappe.defaults @@ -74,15 +74,6 @@ class Database: self.password = password or frappe.conf.db_password self.value_cache = {} - @property - def query(self): - if not hasattr(self, "_query"): - from .query import Query - - self._query = Query() - del Query - return self._query - def setup_type_map(self): pass @@ -599,7 +590,7 @@ class Database: return [map(values.get, fields)] else: - r = self.query.get_sql( + r = frappe.qb.engine.get_query( "Singles", filters={"field": ("in", tuple(fields)), "doctype": doctype}, fields=["field", "value"], @@ -632,7 +623,7 @@ class Database: # Get coulmn and value of the single doctype Accounts Settings account_settings = frappe.db.get_singles_dict("Accounts Settings") """ - queried_result = self.query.get_sql( + queried_result = frappe.qb.engine.get_query( "Singles", filters={"doctype": doctype}, fields=["field", "value"], @@ -705,7 +696,7 @@ class Database: if cache and fieldname in self.value_cache[doctype]: return self.value_cache[doctype][fieldname] - val = self.query.get_sql( + val = frappe.qb.engine.get_query( table="Singles", filters={"doctype": doctype, "field": fieldname}, fields="value", @@ -747,14 +738,7 @@ class Database: ): field_objects = [] - if not isinstance(fields, Criterion): - for field in fields: - if "(" in str(field) or " as " in str(field): - field_objects.append(PseudoColumn(field)) - else: - field_objects.append(field) - - query = self.query.get_sql( + query = frappe.qb.engine.get_query( table=doctype, filters=filters, orderby=order_by, @@ -864,7 +848,7 @@ class Database: frappe.clear_document_cache(dt, docname) else: - query = self.query.build_conditions(table=dt, filters=dn, update=True) + query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True) # TODO: Fix this; doesn't work rn - gavin@frappe.io # frappe.cache().hdel_keys(dt, "document_cache") # Workaround: clear all document caches @@ -937,7 +921,10 @@ class Database: if not key: return defaults - return defaults.get(key) or defaults.get(frappe.scrub(key)) + if key in defaults: + return defaults[key] + + return defaults.get(frappe.scrub(key)) def begin(self): self.sql("START TRANSACTION") @@ -1062,7 +1049,9 @@ class Database: cache_count = frappe.cache().get_value(f"doctype:count:{dt}") if cache_count is not None: return cache_count - query = self.query.get_sql(table=dt, filters=filters, fields=Count("*"), distinct=distinct) + query = frappe.qb.engine.get_query( + table=dt, filters=filters, fields=Count("*"), distinct=distinct + ) count = self.sql(query, debug=debug)[0][0] if not filters and cache: frappe.cache().set_value(f"doctype:count:{dt}", count, expires_in_sec=86400) @@ -1199,7 +1188,7 @@ class Database: Doctype name can be passed directly, it will be pre-pended with `tab`. """ filters = filters or kwargs.get("conditions") - query = self.query.build_conditions(table=doctype, filters=filters).delete() + query = frappe.qb.engine.build_conditions(table=doctype, filters=filters).delete() if "debug" not in kwargs: kwargs["debug"] = debug return query.run(**kwargs) diff --git a/frappe/database/query.py b/frappe/database/query.py index c87117466b..9bb5383b24 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,16 +1,21 @@ import operator import re +from ast import literal_eval from functools import cached_property -from typing import Any, Callable +from types import BuiltinFunctionType +from typing import TYPE_CHECKING, Any, Callable import frappe from frappe import _ -from frappe.boot import get_additional_filters_from_hooks from frappe.model.db_query import get_timespan_date_range -from frappe.query_builder import Criterion, Field, Order, Table +from frappe.query_builder import Criterion, Field, Order, Table, functions +from frappe.query_builder.functions import Function, SqlFunctions TAB_PATTERN = re.compile("^tab") WORDS_PATTERN = re.compile(r"\w+") +BRACKETS_PATTERN = re.compile(r"\(.*?\)|$") +SQL_FUNCTIONS = [sql_function.value for sql_function in SqlFunctions] +COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") def like(key: Field, value: str) -> frappe.qb: @@ -93,7 +98,7 @@ def func_between(key: Field, value: list | tuple) -> frappe.qb: def func_is(key, value): "Wrapper for IS" - return Field(key).isnotnull() if value.lower() == "set" else Field(key).isnull() + return key.isnotnull() if value.lower() == "set" else key.isnull() def func_timespan(key: Field, value: str) -> frappe.qb: @@ -143,6 +148,13 @@ def change_orderby(order: str): return order[0], Order.desc +def literal_eval_(literal): + try: + return literal_eval(literal) + except (ValueError, SyntaxError): + return literal + + # default operators OPERATOR_MAP: dict[str, Callable] = { "+": operator.add, @@ -155,6 +167,8 @@ OPERATOR_MAP: dict[str, Callable] = { "=<": operator.le, ">=": operator.ge, "=>": operator.ge, + "/": operator.truediv, + "*": operator.mul, "in": func_in, "not in": func_not_in, "like": like, @@ -168,11 +182,13 @@ OPERATOR_MAP: dict[str, Callable] = { } -class Query: - tables: dict = {} +class Engine: + tables: dict[str, str] = {} @cached_property def OPERATOR_MAP(self): + from frappe.boot import get_additional_filters_from_hooks + # default operators all_operators = OPERATOR_MAP.copy() @@ -238,7 +254,7 @@ class Query: Returns: conditions (frappe.qb): frappe.qb object """ - if kwargs.get("orderby"): + if kwargs.get("orderby") and kwargs.get("orderby") != "KEEP_DEFAULT_ORDERING": orderby = kwargs.get("orderby") if isinstance(orderby, str) and len(orderby.split()) > 1: for ordby in orderby.split(","): @@ -250,6 +266,7 @@ class Query: if kwargs.get("limit"): conditions = conditions.limit(kwargs.get("limit")) + conditions = conditions.offset(kwargs.get("offset", 0)) if kwargs.get("distinct"): conditions = conditions.distinct() @@ -257,6 +274,9 @@ class Query: if kwargs.get("for_update"): conditions = conditions.for_update() + if kwargs.get("groupby"): + conditions = conditions.groupby(kwargs.get("groupby")) + return conditions def misc_query(self, table: str, filters: list | tuple = None, **kwargs): @@ -306,6 +326,10 @@ class Query: conditions = self.add_conditions(conditions, **kwargs) return conditions + for key, value in filters.items(): + if isinstance(value, bool): + filters.update({key: str(int(value))}) + for key in filters: value = filters.get(key) _operator = self.OPERATOR_MAP["="] @@ -315,7 +339,8 @@ class Query: continue if isinstance(value, (list, tuple)): _operator = self.OPERATOR_MAP[value[0].casefold()] - conditions = conditions.where(_operator(Field(key), value[1])) + _value = value[1] if value[1] else ("",) + conditions = conditions.where(_operator(Field(key), _value)) else: if value is not None: conditions = conditions.where(_operator(Field(key), value)) @@ -352,7 +377,138 @@ class Query: return criterion - def get_sql( + def get_function_object(self, field: str) -> "Function": + """Expects field to look like 'SUM(*)' or 'name' or something similar. Returns PyPika Function object""" + func = field.split("(", maxsplit=1)[0].capitalize() + args_start, args_end = len(func) + 1, field.index(")") + args = field[args_start:args_end].split(",") + + _, alias = field.split(" as ") if " as " in field else (None, None) + + to_cast = "*" not in args + _args = [] + + for arg in args: + initial_fields = literal_eval_(arg.strip()) + if to_cast: + has_primitive_operator = False + for _operator in OPERATOR_MAP.keys(): + if _operator in initial_fields: + operator_mapping = OPERATOR_MAP[_operator] + # Only perform this if operator is of primitive type. + if isinstance(operator_mapping, BuiltinFunctionType): + has_primitive_operator = True + field = operator_mapping( + *map(lambda field: Field(field.strip()), arg.split(_operator)), + ) + + field = Field(initial_fields) if not has_primitive_operator else field + else: + field = initial_fields + + _args.append(field) + try: + return getattr(functions, func)(*_args, alias=alias or None) + except AttributeError: + # Fall back for functions not present in `SqlFunctions`` + return Function(func, *_args, alias=alias or None) + + def function_objects_from_string(self, fields): + fields = list(map(lambda str: str.strip(), COMMA_PATTERN.split(fields))) + return self.function_objects_from_list(fields=fields) + + def function_objects_from_list(self, fields): + functions = [] + for field in fields: + field = field.casefold() if isinstance(field, str) else field + if not issubclass(type(field), Criterion): + if any([f"{func}(" in field for func in SQL_FUNCTIONS]) or "(" in field: + functions.append(field) + + return [self.get_function_object(function) for function in functions] + + def remove_string_functions(self, fields, function_objects): + """Remove string functions from fields which have already been converted to function objects""" + for function in function_objects: + if isinstance(fields, str): + if function.alias: + fields = fields.replace(" as " + function.alias.casefold(), "") + fields = BRACKETS_PATTERN.sub("", fields.replace(function.name.casefold(), "")) + # Check if only comma is left in fields after stripping functions. + if "," in fields and (len(fields.strip()) == 1): + fields = "" + else: + updated_fields = [] + for field in fields: + if isinstance(field, str): + if function.alias: + field = field.replace(" as " + function.alias.casefold(), "") + field = ( + BRACKETS_PATTERN.sub("", field).strip().casefold().replace(function.name.casefold(), "") + ) + updated_fields.append(field) + + fields = [field for field in updated_fields if field] + + return fields + + def set_fields(self, fields, **kwargs): + fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" + if isinstance(fields, list) and None in fields and Field not in fields: + return None + + function_objects = [] + + is_list = isinstance(fields, (list, tuple, set)) + if is_list and len(fields) == 1: + fields = fields[0] + is_list = False + + if is_list: + function_objects += self.function_objects_from_list(fields=fields) + + is_str = isinstance(fields, str) + if is_str: + fields = fields.casefold() + function_objects += self.function_objects_from_string(fields=fields) + + fields = self.remove_string_functions(fields, function_objects) + + if is_str and "," in fields: + fields = [field.replace(" ", "") if "as" not in field else field for field in fields.split(",")] + is_list, is_str = True, False + + if is_str: + if fields == "*": + return fields + if " as " in fields: + fields, reference = fields.split(" as ") + fields = Field(fields).as_(reference) + + if not is_str and fields: + if issubclass(type(fields), Criterion): + return fields + updated_fields = [] + if "*" in fields: + return fields + for field in fields: + if not isinstance(field, Criterion) and field: + if " as " in field: + field, reference = field.split(" as ") + updated_fields.append(Field(field.strip()).as_(reference)) + else: + updated_fields.append(Field(field)) + + fields = updated_fields + + # Need to check instance again since fields modified. + if not isinstance(fields, (list, tuple, set)): + fields = [fields] if fields else [] + + fields.extend(function_objects) + return fields + + def get_query( self, table: str, fields: list | tuple, @@ -362,15 +518,20 @@ class Query: # Clean up state before each query self.tables = {} criterion = self.build_conditions(table, filters, **kwargs) + fields = self.set_fields(kwargs.get("field_objects") or fields, **kwargs) + + join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" if len(self.tables) > 1: primary_table = self.tables[table] del self.tables[table] for table_object in self.tables.values(): - criterion = criterion.left_join(table_object).on(table_object.parent == primary_table.name) + criterion = getattr(criterion, join)(table_object).on( + table_object.parent == primary_table.name + ) if isinstance(fields, (list, tuple)): - query = criterion.select(*kwargs.get("field_objects", fields)) + query = criterion.select(*fields) elif isinstance(fields, Criterion): query = criterion.select(fields) diff --git a/frappe/defaults.py b/frappe/defaults.py index c2f4a3fe56..02076b1fda 100644 --- a/frappe/defaults.py +++ b/frappe/defaults.py @@ -85,18 +85,19 @@ def get_user_permissions(user=None): def get_defaults(user=None): - globald = get_defaults_for() + global_defaults = get_defaults_for() if not user: user = frappe.session.user if frappe.session else "Guest" - if user: - userd = {} - userd.update(get_defaults_for(user)) - userd.update({"user": user, "owner": user}) - globald.update(userd) + if not user: + return global_defaults - return globald + defaults = global_defaults.copy() + defaults.update(get_defaults_for(user)) + defaults.update(user=user, owner=user) + + return defaults def clear_user_default(key, user=None): @@ -241,8 +242,4 @@ def get_defaults_for(parent="__default"): def _clear_cache(parent): - if parent in common_default_keys: - frappe.clear_cache() - else: - clear_notifications(user=parent) - frappe.clear_cache(user=parent) + frappe.clear_cache(user=parent if parent not in common_default_keys else None) diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 12a6105c4b..8e808ff635 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -200,7 +200,7 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if txt: search_conditions = [numberCard[field].like(f"%{txt}%") for field in searchfields] - condition_query = frappe.db.query.build_conditions(doctype, filters) + condition_query = frappe.qb.engine.build_conditions(doctype, filters) return ( condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) diff --git a/frappe/desk/doctype/route_history/route_history.py b/frappe/desk/doctype/route_history/route_history.py index c62311ae02..a576ac73f5 100644 --- a/frappe/desk/doctype/route_history/route_history.py +++ b/frappe/desk/doctype/route_history/route_history.py @@ -4,45 +4,18 @@ import frappe from frappe.deferred_insert import deferred_insert as _deferred_insert from frappe.model.document import Document -from frappe.query_builder import DocType, Interval -from frappe.query_builder.functions import Count, Now class RouteHistory(Document): @staticmethod def clear_old_logs(days=30): + from frappe.query_builder import Interval + from frappe.query_builder.functions import Now + table = frappe.qb.DocType("Route History") frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) -def flush_old_route_records(): - """Deletes all route records except last 500 records per user""" - records_to_keep_limit = 500 - RouteHistory = DocType("Route History") - - users = ( - frappe.qb.from_(RouteHistory) - .select(RouteHistory.user) - .groupby(RouteHistory.user) - .having(Count(RouteHistory.name) > records_to_keep_limit) - ).run(pluck=True) - - for user in users: - last_record_to_keep = frappe.get_all( - "Route History", - filters={"user": user}, - limit_start=500, - fields=["modified"], - order_by="modified desc", - limit=1, - ) - - frappe.db.delete( - "Route History", - {"modified": ("<=", last_record_to_keep[0].modified), "user": user}, - ) - - @frappe.whitelist() def deferred_insert(routes): routes = [ diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index d48a7f3de4..ea6eb6259c 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -36,7 +36,7 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d ToDo = DocType("ToDo") User = DocType("User") count = Count("*").as_("count") - filtered_records = frappe.db.query.build_conditions(doctype, current_filters).select("name") + filtered_records = frappe.qb.engine.build_conditions(doctype, current_filters).select("name") return ( frappe.qb.from_(ToDo) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index 3faa6d84c1..e88a453e64 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import datetime import json import os from datetime import timedelta @@ -384,6 +385,18 @@ def format_duration_fields(data: frappe._dict) -> None: def build_xlsx_data(data, visible_idx, include_indentation, ignore_visible_idx=False): + EXCEL_TYPES = ( + str, + bool, + type(None), + int, + float, + datetime.datetime, + datetime.date, + datetime.time, + datetime.timedelta, + ) + result = [[]] column_widths = [] @@ -407,7 +420,10 @@ def build_xlsx_data(data, visible_idx, include_indentation, ignore_visible_idx=F continue label = column.get("label") fieldname = column.get("fieldname") - cell_value = cstr(row.get(fieldname, row.get(label, ""))) + cell_value = row.get(fieldname, row.get(label, "")) + if not isinstance(cell_value, EXCEL_TYPES): + cell_value = cstr(cell_value) + if cint(include_indentation) and "indent" in row and col_idx == 0: cell_value = (" " * cint(row["indent"])) + cstr(cell_value) row_data.append(cell_value) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index a357126a48..d22009963d 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -1,6 +1,7 @@ frappe.email_defaults = { "GMail": { "email_server": "imap.gmail.com", + "incoming_port": 993, "use_ssl": 1, "enable_outgoing": 1, "smtp_server": "smtp.gmail.com", @@ -66,6 +67,34 @@ frappe.email_defaults_pop = { }; +function oauth_access(frm) { + return frappe.call({ + method: "frappe.email.oauth.oauth_access", + args: { + "email_account": frm.doc.name, + "service": frm.doc.service || "" + }, + callback: function(r) { + if (!r.exc) { + window.open(r.message.url, "_self"); + } + } + }); +} + +function set_default_max_attachment_size(frm, field) { + if (frm.doc.__islocal && !frm.doc[field]) { + frappe.call({ + method: "frappe.core.api.file.get_max_file_size", + callback: function(r) { + if (!r.exc) { + frm.set_value(field, Number(r.message)/(1024*1024)); + } + }, + }); + } +} + frappe.ui.form.on("Email Account", { service: function(frm) { $.each(frappe.email_defaults[frm.doc.service], function(key, value) { @@ -77,6 +106,7 @@ frappe.ui.form.on("Email Account", { }); } frm.events.show_gmail_message_for_less_secure_apps(frm); + frm.events.toggle_auth_method(frm); }, use_imap: function(frm) { @@ -107,6 +137,12 @@ frappe.ui.form.on("Email Account", { }, onload: function(frm) { + if (frappe.utils.get_query_params().successful_authorization === '1') { + frappe.show_alert(__("Successfully Authorized")); + // FIXME: find better alternative + window.history.replaceState(null, "", window.location.pathname); + } + frm.set_df_property("append_to", "only_select", true); frm.set_query("append_to", "frappe.email.doctype.email_account.email_account.get_append_to"); frm.set_query("append_to", "imap_folder", function() { @@ -118,6 +154,8 @@ frappe.ui.form.on("Email Account", { frm.add_child("imap_folder", {"folder_name": "INBOX"}); frm.refresh_field("imap_folder"); } + frm.toggle_display(['auth_method'], frm.doc.service === "GMail"); + set_default_max_attachment_size(frm, "attachment_limit"); }, refresh: function(frm) { @@ -125,6 +163,7 @@ frappe.ui.form.on("Email Account", { frm.events.enable_incoming(frm); frm.events.notify_if_unreplied(frm); frm.events.show_gmail_message_for_less_secure_apps(frm); + frm.events.show_oauth_authorization_message(frm); if (frappe.route_flags.delete_user_from_locals && frappe.route_flags.linked_user) { delete frappe.route_flags.delete_user_from_locals; @@ -132,9 +171,24 @@ frappe.ui.form.on("Email Account", { } }, + after_save(frm) { + if (frm.doc.auth_method === "OAuth" && !frm.doc.refresh_token) { + oauth_access(frm); + } + }, + + toggle_auth_method: function(frm) { + if (frm.doc.service !== "GMail") { + frm.toggle_display(['auth_method'], false); + frm.doc.auth_method = "Basic"; + } else { + frm.toggle_display(['auth_method'], true); + } + }, + show_gmail_message_for_less_secure_apps: function(frm) { frm.dashboard.clear_headline(); - let msg = __("GMail will only work if you enable 2-step authentication and use app-specific password."); + let msg = __("GMail will only work if you enable 2-step authentication and use app-specific password OR use OAuth."); let cta = __("Read the step by step guide here."); msg += ` ${cta}`; if (frm.doc.service==="GMail") { @@ -142,6 +196,18 @@ frappe.ui.form.on("Email Account", { } }, + show_oauth_authorization_message(frm) { + if (frm.doc.auth_method === "OAuth" && !frm.doc.refresh_token) { + let msg = __('OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.') + frm.dashboard.clear_headline(); + frm.dashboard.set_headline_alert(msg, "yellow"); + } + }, + + authorize_api_access: function(frm) { + oauth_access(frm); + }, + email_id:function(frm) { //pull domain and if no matching domain go create one frm.events.update_domain(frm); diff --git a/frappe/email/doctype/email_account/email_account.json b/frappe/email/doctype/email_account/email_account.json index 65053bab3d..ecb5af7378 100644 --- a/frappe/email/doctype/email_account/email_account.json +++ b/frappe/email/doctype/email_account/email_account.json @@ -14,10 +14,14 @@ "domain", "service", "authentication_column", + "auth_method", + "authorize_api_access", "password", "awaiting_password", "ascii_encode_password", "column_break_10", + "refresh_token", + "access_token", "login_id_is_different", "login_id", "mailbox_settings", @@ -44,9 +48,9 @@ "send_notification_to", "outgoing_mail_settings", "enable_outgoing", - "smtp_server", "use_tls", "use_ssl_for_outgoing", + "smtp_server", "smtp_port", "column_break_38", "default_outgoing", @@ -79,7 +83,8 @@ "in_list_view": 1, "label": "Email Address", "options": "Email", - "reqd": 1 + "reqd": 1, + "unique": 1 }, { "default": "0", @@ -87,7 +92,7 @@ "fieldtype": "Check", "hide_days": 1, "hide_seconds": 1, - "label": "Use different login" + "label": "Use different Email ID" }, { "depends_on": "login_id_is_different", @@ -95,9 +100,10 @@ "fieldtype": "Data", "hide_days": 1, "hide_seconds": 1, - "label": "Email Login ID" + "label": "Alternative Email ID" }, { + "depends_on": "eval: doc.auth_method === \"Basic\"", "fieldname": "password", "fieldtype": "Password", "hide_days": 1, @@ -106,6 +112,7 @@ }, { "default": "0", + "depends_on": "eval: doc.auth_method === \"Basic\"", "fieldname": "awaiting_password", "fieldtype": "Check", "hide_days": 1, @@ -114,6 +121,7 @@ }, { "default": "0", + "depends_on": "eval: doc.auth_method === \"Basic\"", "fieldname": "ascii_encode_password", "fieldtype": "Check", "hide_days": 1, @@ -182,7 +190,7 @@ "fieldtype": "Data", "hide_days": 1, "hide_seconds": 1, - "label": "Email Server" + "label": "Incoming Server" }, { "default": "0", @@ -304,7 +312,7 @@ "fieldtype": "Data", "hide_days": 1, "hide_seconds": 1, - "label": "SMTP Server" + "label": "Outgoing Server" }, { "default": "0", @@ -524,7 +532,7 @@ "fieldtype": "Check", "hide_days": 1, "hide_seconds": 1, - "label": "Use SSL for Outgoing" + "label": "Use SSL" }, { "default": "1", @@ -576,12 +584,39 @@ "fieldname": "section_break_25", "fieldtype": "Section Break", "label": "IMAP Details" + }, + { + "depends_on": "eval: doc.service === \"GMail\" && doc.auth_method === \"OAuth\" && !doc.__islocal && !doc.__unsaved", + "fieldname": "authorize_api_access", + "fieldtype": "Button", + "label": "Authorize API Access" + }, + { + "fieldname": "refresh_token", + "fieldtype": "Small Text", + "hidden": 1, + "label": "Refresh Token", + "read_only": 1 + }, + { + "fieldname": "access_token", + "fieldtype": "Small Text", + "hidden": 1, + "label": "Access Token", + "read_only": 1 + }, + { + "default": "Basic", + "fieldname": "auth_method", + "fieldtype": "Select", + "label": "Method", + "options": "Basic\nOAuth" } ], "icon": "fa fa-inbox", "index_web_pages_for_search": 1, "links": [], - "modified": "2021-11-30 09:03:25.728637", + "modified": "2022-07-11 18:34:06.945668", "modified_by": "Administrator", "module": "Email", "name": "Email Account", @@ -603,5 +638,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 -} +} \ No newline at end of file diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 02afe4f4b5..589ddf42f0 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -20,6 +20,7 @@ from frappe.utils import cint, comma_or, cstr, parse_addr, validate_email_addres from frappe.utils.background_jobs import enqueue, get_jobs from frappe.utils.error import raise_error_on_no_output from frappe.utils.jinja import render_template +from frappe.utils.password import decrypt, encrypt from frappe.utils.user import get_system_managers @@ -63,6 +64,7 @@ class EmailAccount(Document): def validate(self): """Validate Email Address and check POP3/IMAP and SMTP connections is enabled.""" + if self.email_id: validate_email_address(self.email_id, True) @@ -76,25 +78,25 @@ class EmailAccount(Document): if self.enable_incoming and self.use_imap and len(self.imap_folder) <= 0: frappe.throw(_("You need to set one IMAP folder for {0}").format(frappe.bold(self.email_id))) - duplicate_email_account = frappe.get_all( - "Email Account", filters={"email_id": self.email_id, "name": ("!=", self.name)} - ) - if duplicate_email_account: - frappe.throw( - _("Email ID must be unique, Email Account already exists for {0}").format( - frappe.bold(self.email_id) - ) - ) - if frappe.local.flags.in_patch or frappe.local.flags.in_test: return - if ( - not self.awaiting_password - and not frappe.local.flags.in_install - and not frappe.local.flags.in_patch - ): - if self.password or self.smtp_server in ("127.0.0.1", "localhost"): + use_oauth = self.auth_method == "OAuth" + + if getattr(self, "service", "") != "GMail" and use_oauth: + self.auth_method = "Basic" + use_oauth = False + + if use_oauth: + # no need for awaiting password for oauth + self.awaiting_password = 0 + + elif self.refresh_token: + # clear access & refresh token + self.refresh_token = self.access_token = None + + if not frappe.local.flags.in_install and not self.awaiting_password: + if self.refresh_token or self.password or self.smtp_server in ("127.0.0.1", "localhost"): if self.enable_incoming: self.get_incoming_server() self.no_failed = 0 @@ -103,7 +105,8 @@ class EmailAccount(Document): self.validate_smtp_conn() else: if self.enable_incoming or (self.enable_outgoing and not self.no_smtp_authentication): - frappe.throw(_("Password is required or select Awaiting Password")) + if not use_oauth: + frappe.throw(_("Password is required or select Awaiting Password")) if self.notify_if_unreplied: if not self.send_notification_to: @@ -111,11 +114,12 @@ class EmailAccount(Document): for e in self.get_unreplied_notification_emails(): validate_email_address(e, True) - for folder in self.imap_folder: - if self.enable_incoming and folder.append_to: - valid_doctypes = [d[0] for d in get_append_to()] - if folder.append_to not in valid_doctypes: - frappe.throw(_("Append To can be one of {0}").format(comma_or(valid_doctypes))) + if self.enable_incoming: + for folder in self.imap_folder: + if folder.append_to: + valid_doctypes = [d[0] for d in get_append_to()] + if folder.append_to not in valid_doctypes: + frappe.throw(_("Append To can be one of {0}").format(comma_or(valid_doctypes))) def validate_smtp_conn(self): if not self.smtp_server: @@ -155,6 +159,7 @@ class EmailAccount(Document): awaiting_password=self.awaiting_password, email_id=self.email_id, enable_outgoing=self.enable_outgoing, + used_oauth=self.auth_method == "OAuth", ) def there_must_be_only_one_default(self): @@ -204,10 +209,14 @@ class EmailAccount(Document): "host": self.email_server, "use_ssl": self.use_ssl, "username": getattr(self, "login_id", None) or self.email_id, + "service": getattr(self, "service", ""), "use_imap": self.use_imap, "email_sync_rule": email_sync_rule, "incoming_port": get_port(self), "initial_sync_count": self.initial_sync_count or 100, + "use_oauth": self.auth_method == "OAuth", + "refresh_token": decrypt(self.refresh_token) if self.refresh_token else None, + "access_token": decrypt(self.access_token) if self.access_token else None, } ) @@ -274,7 +283,9 @@ class EmailAccount(Document): @property def _password(self): - raise_exception = not (self.no_smtp_authentication or frappe.flags.in_test) + raise_exception = not ( + self.auth_method == "OAuth" or self.no_smtp_authentication or frappe.flags.in_test + ) return self.get_password(raise_exception=raise_exception) @property @@ -393,6 +404,9 @@ class EmailAccount(Document): "default": 0, }, "name": {"conf_names": ("email_sender_name",), "default": "Frappe"}, + "auth_method": {"conf_names": ("auth_method"), "default": "Basic"}, + "access_token": {"conf_names": ("mail_access_token")}, + "refresh_token": {"conf_names": ("mail_refresh_token")}, "from_site_config": {"default": True}, } @@ -400,17 +414,27 @@ class EmailAccount(Document): for doc_field_name, d in field_to_conf_name_map.items(): conf_names, default = d.get("conf_names") or [], d.get("default") value = [frappe.conf.get(k) for k in conf_names if frappe.conf.get(k)] - account_details[doc_field_name] = (value and value[0]) or default + + if doc_field_name in ("refresh_token", "access_token"): + account_details[doc_field_name] = value and encrypt(value[0]) + else: + account_details[doc_field_name] = (value and value[0]) or default + return account_details def sendmail_config(self): return { + "email_account": self.name, "server": self.smtp_server, "port": cint(self.smtp_port), "login": getattr(self, "login_id", None) or self.email_id, "password": self._password, "use_ssl": cint(self.use_ssl_for_outgoing), "use_tls": cint(self.use_tls), + "service": getattr(self, "service", ""), + "use_oauth": self.auth_method == "OAuth", + "refresh_token": decrypt(self.refresh_token) if self.refresh_token else None, + "access_token": decrypt(self.access_token) if self.access_token else None, } def get_smtp_server(self): @@ -494,7 +518,7 @@ class EmailAccount(Document): seen_status = messages.get("seen_status", {}).get(uid) if self.email_sync_option != "UNSEEN" or seen_status != "SEEN": # only append the emails with status != 'SEEN' if sync option is set to 'UNSEEN' - mails.append(InboundMail(message, self, uid, seen_status, append_to)) + mails.append(InboundMail(message, self, frappe.safe_decode(uid), seen_status, append_to)) if not self.enable_incoming: return [] @@ -771,15 +795,25 @@ def notify_unreplied(): def pull(now=False): """Will be called via scheduler, pull emails from all enabled Email accounts.""" + if frappe.cache().get_value("workers:no-internet") == True: if test_internet(): frappe.cache().set_value("workers:no-internet", False) else: return - queued_jobs = get_jobs(site=frappe.local.site, key="job_name")[frappe.local.site] - for email_account in frappe.get_list( - "Email Account", filters={"enable_incoming": 1, "awaiting_password": 0} - ): + + doctype = frappe.qb.DocType("Email Account") + email_accounts = ( + frappe.qb.from_(doctype) + .select(doctype.name) + .where(doctype.enable_incoming == 1) + .where( + (doctype.awaiting_password == 0) + | ((doctype.auth_method == "OAuth") & (doctype.refresh_token.isnotnull())) + ) + .run(as_dict=1) + ) + for email_account in email_accounts: if now: pull_from_email_account(email_account.name) @@ -787,6 +821,7 @@ def pull(now=False): # job_name is used to prevent duplicates in queue job_name = f"pull_from_email_account|{email_account.name}" + queued_jobs = get_jobs(site=frappe.local.site, key="job_name")[frappe.local.site] if job_name not in queued_jobs: enqueue( pull_from_email_account, @@ -824,7 +859,9 @@ def get_max_email_uid(email_account): return max_uid -def setup_user_email_inbox(email_account, awaiting_password, email_id, enable_outgoing): +def setup_user_email_inbox( + email_account, awaiting_password, email_id, enable_outgoing, used_oauth +): """setup email inbox for user""" from frappe.core.doctype.user.user import ask_pass_update @@ -835,6 +872,7 @@ def setup_user_email_inbox(email_account, awaiting_password, email_id, enable_ou row.email_id = email_id row.email_account = email_account row.awaiting_password = awaiting_password or 0 + row.used_oauth = used_oauth or 0 row.enable_outgoing = enable_outgoing or 0 user.save(ignore_permissions=True) @@ -867,8 +905,10 @@ def setup_user_email_inbox(email_account, awaiting_password, email_id, enable_ou if update_user_email_settings: UserEmail = frappe.qb.DocType("User Email") frappe.qb.update(UserEmail).set(UserEmail.awaiting_password, (awaiting_password or 0)).set( - UserEmail.enable_outgoing, enable_outgoing - ).where(UserEmail.email_account == email_account).run() + UserEmail.enable_outgoing, (enable_outgoing or 0) + ).set(UserEmail.used_oauth, (used_oauth or 0)).where( + UserEmail.email_account == email_account + ).run() else: users = " and ".join([frappe.bold(user.get("name")) for user in user_names]) @@ -893,10 +933,10 @@ def remove_user_email_inbox(email_account): doc.save(ignore_permissions=True) -@frappe.whitelist(allow_guest=False) -def set_email_password(email_account, user, password): +@frappe.whitelist() +def set_email_password(email_account, password): account = frappe.get_doc("Email Account", email_account) - if account.awaiting_password: + if account.awaiting_password and not account.auth_method == "OAuth": account.awaiting_password = 0 account.password = password try: diff --git a/frappe/email/doctype/email_group_member/email_group_member.json b/frappe/email/doctype/email_group_member/email_group_member.json index b3b87c6211..0e32135b72 100644 --- a/frappe/email/doctype/email_group_member/email_group_member.json +++ b/frappe/email/doctype/email_group_member/email_group_member.json @@ -1,151 +1,70 @@ { - "allow_copy": 0, - "allow_import": 1, - "allow_rename": 0, - "autoname": "hash", - "beta": 0, - "creation": "2015-03-18 06:15:59.321619", - "custom": 0, - "docstatus": 0, - "doctype": "DocType", - "document_type": "Document", - "editable_grid": 0, - "engine": "InnoDB", + "actions": [], + "allow_import": 1, + "autoname": "hash", + "creation": "2015-03-18 06:15:59.321619", + "doctype": "DocType", + "document_type": "Document", + "engine": "InnoDB", + "field_order": [ + "email_group", + "email", + "unsubscribed" + ], "fields": [ { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "email_group", - "fieldtype": "Link", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 1, - "label": "Email Group", - "length": 0, - "no_copy": 0, - "options": "Email Group", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "email_group", + "fieldtype": "Link", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Email Group", + "options": "Email Group", + "reqd": 1, + "search_index": 1 + }, { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "email", - "fieldtype": "Data", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 1, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Email", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 1, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "email", + "fieldtype": "Data", + "in_global_search": 1, + "in_list_view": 1, + "label": "Email", + "reqd": 1 + }, { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "unsubscribed", - "fieldtype": "Check", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 1, - "in_standard_filter": 0, - "label": "Unsubscribed", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 + "default": "0", + "fieldname": "unsubscribed", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Unsubscribed", + "search_index": 1 } - ], - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, - "in_create": 0, - - "is_submittable": 0, - "issingle": 0, - "istable": 0, - "max_attachments": 0, - "modified": "2017-02-17 17:00:42.551806", - "modified_by": "Administrator", - "module": "Email", - "name": "Email Group Member", - "name_case": "", - "owner": "Administrator", + ], + "links": [], + "modified": "2022-07-11 16:38:34.165271", + "modified_by": "Administrator", + "module": "Email", + "name": "Email Group Member", + "naming_rule": "Random", + "owner": "Administrator", "permissions": [ { - "amend": 0, - "apply_user_permissions": 0, - "cancel": 0, - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "if_owner": 0, - "import": 1, - "permlevel": 0, - "print": 1, - "read": 1, - "report": 1, - "role": "Newsletter Manager", - "set_user_permissions": 0, - "share": 1, - "submit": 0, + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "import": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Newsletter Manager", + "share": 1, "write": 1 } - ], - "quick_entry": 1, - "read_only": 0, - "read_only_onload": 0, - "show_name_in_global_search": 0, - "sort_field": "modified", - "sort_order": "DESC", - "title_field": "email", - "track_changes": 1, - "track_seen": 0 + ], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "title_field": "email", + "track_changes": 1 } \ No newline at end of file diff --git a/frappe/email/doctype/email_queue/email_queue.json b/frappe/email/doctype/email_queue/email_queue.json index f251786c90..c9ec374687 100644 --- a/frappe/email/doctype/email_queue/email_queue.json +++ b/frappe/email/doctype/email_queue/email_queue.json @@ -83,7 +83,8 @@ "fieldname": "reference_name", "fieldtype": "Data", "label": "Reference DocName", - "read_only": 1 + "read_only": 1, + "search_index": 1 }, { "fieldname": "communication", @@ -152,10 +153,11 @@ "idx": 1, "in_create": 1, "links": [], - "modified": "2021-04-29 06:33:25.191729", + "modified": "2022-07-12 15:17:37.934316", "modified_by": "Administrator", "module": "Email", "name": "Email Queue", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { @@ -169,5 +171,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 221f3fbb31..eb07be0b38 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -16,6 +16,7 @@ from frappe.core.utils import html2text from frappe.email.doctype.email_account.email_account import EmailAccount from frappe.email.email_body import add_attachment, get_email, get_formatted_html from frappe.email.queue import get_unsubcribed_url, get_unsubscribe_message +from frappe.email.smtp import SMTPServer from frappe.model.document import Document from frappe.query_builder import DocType, Interval from frappe.query_builder.functions import Now @@ -99,7 +100,7 @@ class EmailQueue(Document): def get_email_account(self): if self.email_account: - return frappe.get_doc("Email Account", self.email_account) + return frappe.get_cached_doc("Email Account", self.email_account) return EmailAccount.find_outgoing( match_by_email=self.sender, match_by_doctype=self.reference_doctype @@ -115,12 +116,12 @@ class EmailQueue(Document): return True - def send(self, is_background_task=False): + def send(self, is_background_task: bool = False, smtp_server_instance: SMTPServer = None): """Send emails to recipients.""" if not self.can_send_now(): return - with SendMailContext(self, is_background_task) as ctx: + with SendMailContext(self, is_background_task, smtp_server_instance) as ctx: message = None for recipient in self.recipients: if not recipient.is_mail_to_be_sent(): @@ -169,21 +170,32 @@ class EmailQueue(Document): @task(queue="short") -def send_mail(email_queue_name, is_background_task=False): - """This is equalent to EmqilQueue.send. +def send_mail(email_queue_name, is_background_task=False, smtp_server_instance: SMTPServer = None): + """This is equivalent to EmailQueue.send. This provides a way to make sending mail as a background job. """ record = EmailQueue.find(email_queue_name) - record.send(is_background_task=is_background_task) + record.send(is_background_task=is_background_task, smtp_server_instance=smtp_server_instance) class SendMailContext: - def __init__(self, queue_doc: Document, is_background_task: bool = False): + def __init__( + self, + queue_doc: Document, + is_background_task: bool = False, + smtp_server_instance: SMTPServer = None, + ): self.queue_doc: EmailQueue = queue_doc self.is_background_task = is_background_task self.email_account_doc = queue_doc.get_email_account() - self.smtp_server = self.email_account_doc.get_smtp_server() + + self.smtp_server = smtp_server_instance or self.email_account_doc.get_smtp_server() + + # if smtp_server_instance is passed, then retain smtp session + # Note: smtp session will have to be manually closed + self.retain_smtp_session = bool(smtp_server_instance) + self.sent_to = [rec.recipient for rec in self.queue_doc.recipients if rec.is_main_sent()] def __enter__(self): @@ -200,11 +212,13 @@ class SendMailContext: JobTimeoutException, ] - self.smtp_server.quit() + if not self.retain_smtp_session: + self.smtp_server.quit() + self.log_exception(exc_type, exc_val, exc_tb) if exc_type in exceptions: - email_status = (self.sent_to and "Partially Sent") or "Not Sent" + email_status = "Partially Sent" if self.sent_to else "Not Sent" self.queue_doc.update_status(status=email_status, commit=True) elif exc_type: if self.queue_doc.retry < get_email_retry_limit(): @@ -216,12 +230,12 @@ class SendMailContext: email_status = self.is_mail_sent_to_all() and "Sent" email_status = email_status or (self.sent_to and "Partially Sent") or "Not Sent" - update_fields = {"status": email_status} - if self.email_account_doc.is_exists_in_db(): - update_fields["email_account"] = self.email_account_doc.name - else: - update_fields["email_account"] = None - + update_fields = { + "status": email_status, + "email_account": self.email_account_doc.name + if self.email_account_doc.is_exists_in_db() + else None, + } self.queue_doc.update_status(**update_fields, commit=True) def log_exception(self, exc_type, exc_val, exc_tb): @@ -249,6 +263,7 @@ class SendMailContext: return Parser(policy=SMTPUTF8).parsestr(message) def message_placeholder(self, placeholder_key): + # sourcery skip: avoid-builtin-shadow map = { "tracker": "", "unsubscribe_url": "", @@ -269,7 +284,7 @@ class SendMailContext: ) message = message.replace(self.message_placeholder("cc"), self.get_receivers_str()) message = message.replace( - self.message_placeholder("recipient"), self.get_receipient_str(recipient_email) + self.message_placeholder("recipient"), self.get_recipient_str(recipient_email) ) message = self.include_attachments(message) return message @@ -304,14 +319,11 @@ class SendMailContext: to_str = ", ".join(self.queue_doc.to) cc_str = ", ".join(self.queue_doc.cc) message = f"This email was sent to {to_str}" - message = message + f" and copied to {cc_str}" if cc_str else message + message = f"{message} and copied to {cc_str}" if cc_str else message return message - def get_receipient_str(self, recipient_email): - message = "" - if self.queue_doc.expose_recipients != "header": - message = recipient_email - return message + def get_recipient_str(self, recipient_email): + return recipient_email if self.queue_doc.expose_recipients != "header" else "" def include_attachments(self, message): message_obj = self.get_message_object(message) @@ -628,7 +640,6 @@ class QueueBuilder: if not (final_recipients + self.final_cc()): return [] - email_queues = [] queue_data = self.as_dict(include_recipients=False) if not queue_data: return [] @@ -636,17 +647,35 @@ class QueueBuilder: if not queue_separately: recipients = list(set(final_recipients + self.final_cc() + self.bcc)) q = EmailQueue.new({**queue_data, **{"recipients": recipients}}, ignore_permissions=True) - email_queues.append(q) + send_now and q.send() else: - for r in final_recipients: - recipients = [r] if email_queues else list(set([r] + self.final_cc() + self.bcc)) - q = EmailQueue.new({**queue_data, **{"recipients": recipients}}, ignore_permissions=True) - email_queues.append(q) + if send_now and len(final_recipients) >= 1000: + # force queueing if there are too many recipients to avoid timeouts + send_now = False + for recipients in frappe.utils.create_batch(final_recipients, 1000): + frappe.enqueue( + self.send_emails, + queue_data=queue_data, + final_recipients=recipients, + job_name=frappe.utils.get_job_name( + "send_bulk_emails_for", self.reference_doctype, self.reference_name + ), + now=frappe.flags.in_test or send_now, + queue="long", + ) - if send_now: - for doc in email_queues: - doc.send() - return email_queues + def send_emails(self, queue_data, final_recipients): + # This is used to bulk send emails from same sender to multiple recipients separately + # This re-uses smtp server instance to minimize the cost of new session creation + smtp_server_instance = None + for r in final_recipients: + recipients = list(set([r] + self.final_cc() + self.bcc)) + q = EmailQueue.new({**queue_data, **{"recipients": recipients}}, ignore_permissions=True) + if not smtp_server_instance: + email_account = q.get_email_account() + smtp_server_instance = email_account.get_smtp_server() + q.send(smtp_server_instance=smtp_server_instance) + smtp_server_instance.quit() def as_dict(self, include_recipients=True): email_account = self.get_outgoing_email_account() diff --git a/frappe/email/doctype/email_queue_recipient/email_queue_recipient.json b/frappe/email/doctype/email_queue_recipient/email_queue_recipient.json index 8d1f77e818..c217886ce6 100644 --- a/frappe/email/doctype/email_queue_recipient/email_queue_recipient.json +++ b/frappe/email/doctype/email_queue_recipient/email_queue_recipient.json @@ -1,122 +1,46 @@ { - "allow_copy": 0, - "allow_import": 0, - "allow_rename": 0, - "beta": 0, - "creation": "2016-12-08 12:01:07.993900", - "custom": 0, - "docstatus": 0, - "doctype": "DocType", - "document_type": "", - "editable_grid": 0, - "engine": "InnoDB", + "actions": [], + "creation": "2016-12-08 12:01:07.993900", + "doctype": "DocType", + "engine": "InnoDB", + "field_order": [ + "recipient", + "status", + "error" + ], "fields": [ { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "recipient", - "fieldtype": "Data", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_list_view": 1, - "label": "Recipient", - "length": 0, - "no_copy": 0, - "options": "Email", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "fieldname": "recipient", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Recipient", + "options": "Email" + }, { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "default": "Not Sent", - "fieldname": "status", - "fieldtype": "Select", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_list_view": 1, - "label": "Status", - "length": 0, - "no_copy": 0, - "options": "\nNot Sent\nSending\nSent\nError\nExpired", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 - }, + "default": "Not Sent", + "fieldname": "status", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Status", + "options": "\nNot Sent\nSending\nSent\nError\nExpired", + "search_index": 1 + }, { - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, - "fieldname": "error", - "fieldtype": "Code", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_list_view": 0, - "label": "Error", - "length": 0, - "no_copy": 0, - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 0, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "unique": 0 + "fieldname": "error", + "fieldtype": "Code", + "label": "Error" } - ], - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, - "in_create": 0, - - "is_submittable": 0, - "issingle": 0, - "istable": 1, - "max_attachments": 0, - "modified": "2016-12-08 14:05:33.578240", - "modified_by": "Administrator", - "module": "Email", - "name": "Email Queue Recipient", - "name_case": "", - "owner": "Administrator", - "permissions": [], - "quick_entry": 1, - "read_only": 0, - "read_only_onload": 0, - "sort_field": "modified", - "sort_order": "DESC", - "track_seen": 0 + ], + "istable": 1, + "links": [], + "modified": "2022-07-11 16:38:10.644417", + "modified_by": "Administrator", + "module": "Email", + "name": "Email Queue Recipient", + "owner": "Administrator", + "permissions": [], + "quick_entry": 1, + "sort_field": "modified", + "sort_order": "DESC", + "states": [] } \ No newline at end of file diff --git a/frappe/email/doctype/newsletter/newsletter.js b/frappe/email/doctype/newsletter/newsletter.js index 55805ad485..3c52e61cbb 100644 --- a/frappe/email/doctype/newsletter/newsletter.js +++ b/frappe/email/doctype/newsletter/newsletter.js @@ -30,12 +30,12 @@ frappe.ui.form.on('Newsletter', { frm.add_custom_button(__('Send now'), () => { if (frm.doc.schedule_send) { frappe.confirm(__("This newsletter was scheduled to send on a later date. Are you sure you want to send it now?"), function () { - frm.call('send_emails').then(() => frm.refresh()); + frm.events.send_emails(frm); }); return; } - frappe.confirm(__("Are you sure you want to send this newsletter now?"), function () { - frm.call('send_emails').then(() => frm.refresh()); + frappe.confirm(__("Are you sure you want to send this newsletter now?"), () => { + frm.events.send_emails(frm); }); }, __('Send')); @@ -44,8 +44,7 @@ frappe.ui.form.on('Newsletter', { }, __('Send')); } - frm.events.setup_dashboard(frm); - frm.events.setup_sending_status(frm); + frm.events.update_sending_status(frm); if (frm.is_new() && !doc.sender_email) { let { fullname, email } = frappe.user_info(doc.owner); @@ -56,6 +55,15 @@ frappe.ui.form.on('Newsletter', { frm.trigger('update_schedule_message'); }, + send_emails(frm) { + frappe.dom.freeze(__("Queuing emails...")); + frm.call('send_emails').then(() => { + frm.refresh(); + frappe.dom.unfreeze(); + frappe.show_alert(__("Queued {0} emails", [frappe.utils.shorten_number(frm.doc.total_recipients)])); + }); + }, + schedule_send_dialog(frm) { let hours = frappe.utils.range(24); let time_slots = hours.map(hour => { @@ -128,77 +136,36 @@ frappe.ui.form.on('Newsletter', { d.show(); }, - setup_dashboard(frm) { - if (!frm.doc.__islocal && cint(frm.doc.email_sent) - && frm.doc.__onload && frm.doc.__onload.status_count) { - var stat = frm.doc.__onload.status_count; - var total = frm.doc.scheduled_to_send; - if (total) { - $.each(stat, function (k, v) { - stat[k] = flt(v * 100 / total, 2) + '%'; - }); - - frm.dashboard.add_progress("Status", [ - { - title: stat["Not Sent"] + " Queued", - width: stat["Not Sent"], - progress_class: "progress-bar-info" - }, - { - title: stat["Sent"] + " Sent", - width: stat["Sent"], - progress_class: "progress-bar-success" - }, - { - title: stat["Sending"] + " Sending", - width: stat["Sending"], - progress_class: "progress-bar-warning" - }, - { - title: stat["Error"] + "% Error", - width: stat["Error"], - progress_class: "progress-bar-danger" - } - ]); - } - } - }, - - setup_sending_status(frm) { - frm.call('get_sending_status').then(r => { - if (r.message) { - frm.events.update_sending_progress(frm, r.message.sent, r.message.total); - } - if (r.message.sent >= r.message.total) { + async update_sending_status(frm) { + if (frm.doc.email_sent && frm.$wrapper.is(':visible') && !frm.waiting_for_request) { + frm.waiting_for_request = true; + let res = await frm.call('get_sending_status'); + frm.waiting_for_request = false; + let stats = res.message; + stats && frm.events.update_sending_progress(frm, stats); + if (stats.sent + stats.error >= frm.doc.total_recipients || (!stats.total && !stats.emails_queued)) { + frm.sending_status && clearInterval(frm.sending_status); + frm.sending_status = null; return; } - if (frm.sending_status) return; + } - frm.sending_status = setInterval(() => { - if (frm.doc.email_sent && frm.$wrapper.is(':visible')) { - frm.call('get_sending_status').then(r => { - if (r.message) { - let { sent, total } = r.message; - frm.events.update_sending_progress(frm, sent, total); - - if (sent >= total) { - clearInterval(frm.sending_status); - frm.sending_status = null; - return; - } - } - }); - } - }, 5000); - }); + if (frm.sending_status) return; + frm.sending_status = setInterval(() => frm.events.update_sending_status(frm), 5000); }, - update_sending_progress(frm, sent, total) { - if (sent >= total) { + update_sending_progress(frm, stats) { + if (stats.sent + stats.error >= frm.doc.total_recipients || !frm.doc.email_sent) { + frm.doc.email_sent && frm.page.set_indicator(__("Sent"), "green"); frm.dashboard.hide_progress(); return; } - frm.dashboard.show_progress(__('Sending emails'), sent * 100 / total, __("{0} of {1} sent", [sent, total])); + if (stats.total) { + frm.page.set_indicator(__("Sending"), "blue"); + frm.dashboard.show_progress(__('Sending emails'), stats.sent * 100 / frm.doc.total_recipients, __("{0} of {1} sent", [stats.sent, frm.doc.total_recipients])); + } else if (stats.emails_queued) { + frm.page.set_indicator(__("Queued"), "blue"); + } }, on_hide(frm) { diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 757a8c875b..b0cbb1993d 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -6,6 +6,7 @@ import frappe import frappe.utils from frappe import _ from frappe.email.doctype.email_group.email_group import add_subscribers +from frappe.utils.safe_exec import is_job_queued from frappe.utils.verified_command import get_signed_params, verify_request from frappe.website.website_generator import WebsiteGenerator @@ -35,13 +36,19 @@ class Newsletter(WebsiteGenerator): order_by="status", ) sent = 0 + error = 0 total = 0 for row in count_by_status: if row.status == "Sent": sent = row.count + elif row.status == "Error": + error = row.count total += row.count - - return {"sent": sent, "total": total} + emails_queued = is_job_queued( + job_name=frappe.utils.get_job_name("send_bulk_emails_for", self.doctype, self.name), + queue="long", + ) + return {"sent": sent, "error": error, "total": total, "emails_queued": emails_queued} @frappe.whitelist() def send_test_email(self, email): @@ -75,7 +82,6 @@ class Newsletter(WebsiteGenerator): self.schedule_sending = False self.schedule_send = None self.queue_all() - frappe.msgprint(_("Email queued to {0} recipients").format(self.total_recipients)) def validate_send(self): """Validate if Newsletter can be sent.""" @@ -140,7 +146,8 @@ class Newsletter(WebsiteGenerator): """Get list of pending recipients of the newsletter. These recipients may not have receive the newsletter in the previous iteration. """ - return [x for x in self.newsletter_recipients if x not in self.get_success_recipients()] + success_recipients = set(self.get_success_recipients()) + return [x for x in self.newsletter_recipients if x not in success_recipients] def queue_all(self): """Queue Newsletter to all the recipients generated from the `Email Group` table""" diff --git a/frappe/email/doctype/newsletter/newsletter_list.js b/frappe/email/doctype/newsletter/newsletter_list.js index 9ded6148e0..0b82f1c9e4 100644 --- a/frappe/email/doctype/newsletter/newsletter_list.js +++ b/frappe/email/doctype/newsletter/newsletter_list.js @@ -4,9 +4,9 @@ frappe.listview_settings['Newsletter'] = { if (doc.email_sent) { return [__("Sent"), "green", "email_sent,=,Yes"]; } else if (doc.schedule_sending) { - return [__("Scheduled"), "orange", "email_sent,=,No|schedule_sending,=,Yes"]; + return [__("Scheduled"), "purple", "email_sent,=,No|schedule_sending,=,Yes"]; } else { - return [__("Not Sent"), "orange", "email_sent,=,No"]; + return [__("Not Sent"), "gray", "email_sent,=,No"]; } } }; diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index b493ca2cb5..20f81cb89b 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -267,6 +267,7 @@ class EMail: validate_email_address(strip(self.sender), True) self.reply_to = validate_email_address(strip(self.reply_to) or self.sender, True) + self.set_header("X-Original-From", self.sender) self.replace_sender() self.replace_sender_name() @@ -279,16 +280,14 @@ class EMail: def replace_sender(self): if cint(self.email_account.always_use_account_email_id_as_sender): - self.set_header("X-Original-From", self.sender) - sender_name, sender_email = parse_addr(self.sender) + sender_name, _ = parse_addr(self.sender) self.sender = email.utils.formataddr( (str(Header(sender_name or self.email_account.name, "utf-8")), self.email_account.email_id) ) def replace_sender_name(self): if cint(self.email_account.always_use_account_name_as_sender_name): - self.set_header("X-Original-From", self.sender) - sender_name, sender_email = parse_addr(self.sender) + _, sender_email = parse_addr(self.sender) self.sender = email.utils.formataddr( (str(Header(self.email_account.name, "utf-8")), sender_email) ) diff --git a/frappe/email/oauth.py b/frappe/email/oauth.py new file mode 100644 index 0000000000..46d1565275 --- /dev/null +++ b/frappe/email/oauth.py @@ -0,0 +1,163 @@ +import base64 +from imaplib import IMAP4 +from poplib import POP3 +from smtplib import SMTP +from urllib.parse import quote + +import frappe +from frappe.integrations.google_oauth import GoogleOAuth +from frappe.utils.password import encrypt + + +class OAuthenticationError(Exception): + pass + + +class Oauth: + def __init__( + self, + conn: IMAP4 | POP3 | SMTP, + email_account: str, + email: str, + access_token: str, + refresh_token: str, + service: str, + mechanism: str = "XOAUTH2", + ) -> None: + + self.email_account = email_account + self.email = email + self.service = service + self._mechanism = mechanism + self._conn = conn + self._access_token = access_token + self._refresh_token = refresh_token + + self._validate() + + def _validate(self) -> None: + if self.service != "GMail": + raise NotImplementedError( + f"Service {self.service} currently doesn't have oauth implementation." + ) + + if not self._refresh_token: + frappe.throw( + frappe._("Please Authorize OAuth."), + OAuthenticationError, + frappe._("OAuth Error"), + ) + + @property + def _auth_string(self) -> str: + return f"user={self.email}\1auth=Bearer {self._access_token}\1\1" + + def connect(self, _retry: int = 0) -> None: + """Connection method with retry on exception for Oauth""" + try: + if isinstance(self._conn, POP3): + res = self._connect_pop() + + if not res.startswith(b"+OK"): + raise + + elif isinstance(self._conn, IMAP4): + self._connect_imap() + + else: + # SMTP + self._connect_smtp() + + except Exception as e: + # maybe the access token expired - refreshing + access_token = self._refresh_access_token() + + if not access_token or _retry > 0: + frappe.log_error( + "OAuth Error - Authentication Failed", str(e), "Email Account", self.email_account + ) + # raising a bare exception here as we have a lot of exception handling present + # where the connect method is called from - hence just logging and raising. + raise + + self._access_token = access_token + self.connect(_retry + 1) + + def _connect_pop(self) -> bytes: + # poplib doesn't have AUTH command implementation + res = self._conn._shortcmd( + "AUTH {} {}".format( + self._mechanism, base64.b64encode(bytes(self._auth_string, "utf-8")).decode("utf-8") + ) + ) + + return res + + def _connect_imap(self) -> None: + self._conn.authenticate(self._mechanism, lambda x: self._auth_string) + + def _connect_smtp(self) -> None: + self._conn.auth(self._mechanism, lambda x: self._auth_string, initial_response_ok=False) + + def _refresh_access_token(self) -> str: + """Refreshes access token via calling `refresh_access_token` method of oauth service object""" + service_obj = self._get_service_object() + access_token = service_obj.refresh_access_token(self._refresh_token).get("access_token", None) + + # set the new access token in db + frappe.db.set_value( + "Email Account", self.email_account, "access_token", access_token, update_modified=False + ) + return access_token + + def _get_service_object(self): + """Get Oauth service object""" + + return { + "GMail": GoogleOAuth("mail", validate=False), + }[self.service] + + +@frappe.whitelist(methods=["POST"]) +def oauth_access(email_account: str, service: str): + """Used as a default endpoint/caller for all oauth services. + Returns authorization url for redirection""" + + if not service: + frappe.throw(frappe._("No Service is selected. Please select one and try again!")) + + doctype = "Email Account" + + if service == "GMail": + return authorize_google_access(email_account, doctype) + + raise NotImplementedError(f"Service {service} currently doesn't have oauth implementation.") + + +def authorize_google_access(email_account, doctype: str = "Email Account", code: str = None): + """Facilitates google oauth for email. + This is invoked 2 times - first time when user clicks `Authorze API Access` for getting the authorization url + and second time for setting the refresh and access token in db when google redirects back with oauth code.""" + + oauth_obj = GoogleOAuth("mail") + + if not code: + return oauth_obj.get_authentication_url( + { + "method": "frappe.email.oauth.authorize_google_access", + "redirect": f"/app/Form/{quote(doctype)}/{quote(email_account)}", + "success_query_param": "successful_authorization=1", + "email_account": email_account, + }, + ) + + res = oauth_obj.authorize(code) + frappe.db.set_value( + doctype, + email_account, + { + "refresh_token": encrypt(res.get("refresh_token")), + "access_token": encrypt(res.get("access_token")), + }, + update_modified=False, + ) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 93e1a68285..e26748dd07 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -18,6 +18,7 @@ from email_reply_parser import EmailReplyParser import frappe from frappe import _, safe_decode, safe_encode from frappe.core.doctype.file import MaxFileSizeReachedError, get_random_filename +from frappe.email.oauth import Oauth from frappe.utils import ( add_days, cint, @@ -98,7 +99,20 @@ class EmailServer: self.imap = Timed_IMAP4( self.settings.host, self.settings.incoming_port, timeout=frappe.conf.get("pop_timeout") ) - self.imap.login(self.settings.username, self.settings.password) + + if self.settings.use_oauth: + Oauth( + self.imap, + self.settings.email_account, + self.settings.username, + self.settings.access_token, + self.settings.refresh_token, + self.settings.service, + ).connect() + + else: + self.imap.login(self.settings.username, self.settings.password) + # connection established! return True @@ -119,8 +133,19 @@ class EmailServer: self.settings.host, self.settings.incoming_port, timeout=frappe.conf.get("pop_timeout") ) - self.pop.user(self.settings.username) - self.pop.pass_(self.settings.password) + if self.settings.use_oauth: + Oauth( + self.pop, + self.settings.email_account, + self.settings.username, + self.settings.access_token, + self.settings.refresh_token, + self.settings.service, + ).connect() + + else: + self.pop.user(self.settings.username) + self.pop.pass_(self.settings.password) # connection established! return True diff --git a/frappe/email/smtp.py b/frappe/email/smtp.py index 1211419de1..10eb2f7681 100644 --- a/frappe/email/smtp.py +++ b/frappe/email/smtp.py @@ -5,6 +5,7 @@ import smtplib import frappe from frappe import _ +from frappe.email.oauth import Oauth from frappe.utils import cint, cstr @@ -43,13 +44,31 @@ def send(email, append_to=None, retry=1): class SMTPServer: - def __init__(self, server, login=None, password=None, port=None, use_tls=None, use_ssl=None): + def __init__( + self, + server, + login=None, + email_account=None, + password=None, + port=None, + use_tls=None, + use_ssl=None, + use_oauth=0, + refresh_token=None, + access_token=None, + service=None, + ): self.login = login + self.email_account = email_account self.password = password self._server = server self._port = port self.use_tls = use_tls self.use_ssl = use_ssl + self.use_oauth = use_oauth + self.refresh_token = refresh_token + self.access_token = access_token + self.service = service self._session = None if not self.server: @@ -91,7 +110,13 @@ class SMTPServer: ) self.secure_session(_session) - if self.login and self.password: + + if self.use_oauth: + Oauth( + _session, self.email_account, self.login, self.access_token, self.refresh_token, self.service + ).connect() + + elif self.password: res = _session.login(str(self.login or ""), str(self.password or "")) # check if logged correctly @@ -122,7 +147,7 @@ class SMTPServer: @classmethod def throw_invalid_credentials_exception(cls): frappe.throw( - _("Incorrect email or password. Please check your login credentials."), + _("Please check your email login credentials."), title=_("Invalid Credentials"), exc=InvalidEmailCredentials, ) diff --git a/frappe/email/test_email_body.py b/frappe/email/test_email_body.py index 3ff4245924..d5b1013a73 100644 --- a/frappe/email/test_email_body.py +++ b/frappe/email/test_email_body.py @@ -5,6 +5,7 @@ import base64 import os import unittest +import frappe from frappe import safe_decode from frappe.email.doctype.email_queue.email_queue import QueueBuilder, SendMailContext from frappe.email.email_body import ( @@ -54,26 +55,27 @@ This is the text version of this email uni_chr1 = chr(40960) uni_chr2 = chr(1972) - queue_doc = QueueBuilder( + QueueBuilder( recipients=["test@example.com"], sender="me@example.com", subject="Test Subject", - message="

" + uni_chr1 + "abcd" + uni_chr2 + "

", + message=f"

{uni_chr1}abcd{uni_chr2}

", text_content="whatever", - ).process()[0] + ).process() + queue_doc = frappe.get_last_doc("Email Queue") mail_ctx = SendMailContext(queue_doc=queue_doc) result = mail_ctx.build_message(recipient_email="test@test.com") self.assertTrue(b"

=EA=80=80abcd=DE=B4

" in result) def test_prepare_message_returns_cr_lf(self): - queue_doc = QueueBuilder( + QueueBuilder( recipients=["test@example.com"], sender="me@example.com", subject="Test Subject", message="

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

", text_content="whatever", - ).process()[0] - + ).process() + queue_doc = frappe.get_last_doc("Email Queue") mail_ctx = SendMailContext(queue_doc=queue_doc) result = safe_decode(mail_ctx.build_message(recipient_email="test@test.com")) diff --git a/frappe/email/utils.py b/frappe/email/utils.py index 147284a625..7fc2e0ff89 100644 --- a/frappe/email/utils.py +++ b/frappe/email/utils.py @@ -1,5 +1,6 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and contributors # License: MIT. See LICENSE + import imaplib import poplib diff --git a/frappe/handler.py b/frappe/handler.py index d74125a9e0..cee6d3fbde 100644 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -206,7 +206,7 @@ def upload_file(): frappe.local.uploaded_file = content frappe.local.uploaded_filename = filename - if (not file_url or content) and ( + if content is not None and ( frappe.session.user == "Guest" or (user and not user.has_desk_access()) ): filetype = guess_type(filename)[0] diff --git a/frappe/hooks.py b/frappe/hooks.py index 54d068fa9d..a337d8e0d3 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -242,7 +242,6 @@ scheduler_events = { "weekly_long": [ "frappe.integrations.doctype.dropbox_settings.dropbox_settings.take_backups_weekly", "frappe.integrations.doctype.s3_backup_settings.s3_backup_settings.take_backups_weekly", - "frappe.desk.doctype.route_history.route_history.flush_old_route_records", "frappe.desk.form.document_follow.send_weekly_updates", "frappe.social.doctype.energy_point_log.energy_point_log.send_weekly_summary", "frappe.integrations.doctype.google_drive.google_drive.weekly_backup", diff --git a/frappe/integrations/doctype/google_calendar/google_calendar.py b/frappe/integrations/doctype/google_calendar/google_calendar.py index d8dc7fab1d..09ed012454 100644 --- a/frappe/integrations/doctype/google_calendar/google_calendar.py +++ b/frappe/integrations/doctype/google_calendar/google_calendar.py @@ -13,7 +13,7 @@ from googleapiclient.errors import HttpError import frappe from frappe import _ -from frappe.integrations.doctype.google_settings.google_settings import get_auth_url +from frappe.integrations.google_oauth import GoogleOAuth from frappe.model.document import Document from frappe.utils import ( add_days, @@ -90,7 +90,7 @@ class GoogleCalendar(Document): } try: - r = requests.post(get_auth_url(), data=data).json() + r = requests.post(GoogleOAuth.OAUTH_URL, data=data).json() except requests.exceptions.HTTPError: button_label = frappe.bold(_("Allow Google Calendar Access")) frappe.throw( @@ -130,7 +130,7 @@ def authorize_access(g_calendar, reauthorize=None): "redirect_uri": redirect_uri, "grant_type": "authorization_code", } - r = requests.post(get_auth_url(), data=data).json() + r = requests.post(GoogleOAuth.OAUTH_URL, data=data).json() if "refresh_token" in r: frappe.db.set_value( @@ -191,7 +191,7 @@ def get_google_calendar_object(g_calendar): credentials_dict = { "token": account.get_access_token(), "refresh_token": account.get_password(fieldname="refresh_token", raise_exception=False), - "token_uri": get_auth_url(), + "token_uri": GoogleOAuth.OAUTH_URL, "client_id": google_settings.client_id, "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), "scopes": "https://www.googleapis.com/auth/calendar/v3", diff --git a/frappe/integrations/doctype/google_contacts/google_contacts.js b/frappe/integrations/doctype/google_contacts/google_contacts.js index 7cbef46699..6e8035f38d 100644 --- a/frappe/integrations/doctype/google_contacts/google_contacts.js +++ b/frappe/integrations/doctype/google_contacts/google_contacts.js @@ -37,16 +37,11 @@ frappe.ui.form.on('Google Contacts', { } }, authorize_google_contacts_access: function(frm) { - let reauthorize = 0; - if(frm.doc.authorization_code) { - reauthorize = 1; - } - frappe.call({ method: "frappe.integrations.doctype.google_contacts.google_contacts.authorize_access", args: { "g_contact": frm.doc.name, - "reauthorize": reauthorize + "reauthorize": frm.doc.authorization_code ? 1 : 0 }, callback: function(r) { if(!r.exc) { diff --git a/frappe/integrations/doctype/google_contacts/google_contacts.py b/frappe/integrations/doctype/google_contacts/google_contacts.py index 5e4869be43..c1f445b599 100644 --- a/frappe/integrations/doctype/google_contacts/google_contacts.py +++ b/frappe/integrations/doctype/google_contacts/google_contacts.py @@ -2,18 +2,14 @@ # License: MIT. See LICENSE -import google.oauth2.credentials -import requests -from googleapiclient.discovery import build +from urllib.parse import quote + from googleapiclient.errors import HttpError import frappe from frappe import _ -from frappe.integrations.doctype.google_settings.google_settings import get_auth_url +from frappe.integrations.google_oauth import GoogleOAuth from frappe.model.document import Document -from frappe.utils import get_request_site_address - -SCOPES = "https://www.googleapis.com/auth/contacts" class GoogleContacts(Document): @@ -22,120 +18,57 @@ class GoogleContacts(Document): frappe.throw(_("Enable Google API in Google Settings.")) def get_access_token(self): - google_settings = frappe.get_doc("Google Settings") - - if not google_settings.enable: - frappe.throw(_("Google Contacts Integration is disabled.")) - if not self.refresh_token: button_label = frappe.bold(_("Allow Google Contacts Access")) raise frappe.ValidationError(_("Click on {0} to generate Refresh Token.").format(button_label)) - data = { - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "refresh_token": self.get_password(fieldname="refresh_token", raise_exception=False), - "grant_type": "refresh_token", - "scope": SCOPES, - } - - try: - r = requests.post(get_auth_url(), data=data).json() - except requests.exceptions.HTTPError: - button_label = frappe.bold(_("Allow Google Contacts Access")) - frappe.throw( - _( - "Something went wrong during the token generation. Click on {0} to generate a new one." - ).format(button_label) - ) + oauth_obj = GoogleOAuth("contacts") + r = oauth_obj.refresh_access_token( + self.get_password(fieldname="refresh_token", raise_exception=False) + ) return r.get("access_token") -@frappe.whitelist() -def authorize_access(g_contact, reauthorize=None): +@frappe.whitelist(methods=["POST"]) +def authorize_access(g_contact, reauthorize=False, code=None): """ If no Authorization code get it from Google and then request for Refresh Token. Google Contact Name is set to flags to set_value after Authorization Code is obtained. """ - google_settings = frappe.get_doc("Google Settings") - google_contact = frappe.get_doc("Google Contacts", g_contact) - - redirect_uri = ( - get_request_site_address(True) - + "?cmd=frappe.integrations.doctype.google_contacts.google_contacts.google_callback" + oauth_code = ( + frappe.db.get_value("Google Contacts", g_contact, "authorization_code") if not code else code ) + oauth_obj = GoogleOAuth("contacts") - if not google_contact.authorization_code or reauthorize: - frappe.cache().hset("google_contacts", "google_contact", google_contact.name) - return get_authentication_url(client_id=google_settings.client_id, redirect_uri=redirect_uri) - else: - try: - data = { - "code": google_contact.authorization_code, - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password( - fieldname="client_secret", raise_exception=False - ), - "redirect_uri": redirect_uri, - "grant_type": "authorization_code", - } - r = requests.post(get_auth_url(), data=data).json() - - if "refresh_token" in r: - frappe.db.set_value( - "Google Contacts", google_contact.name, "refresh_token", r.get("refresh_token") - ) - frappe.db.commit() - - frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = f"/app/Form/Google%20Contacts/{google_contact.name}" - - frappe.msgprint(_("Google Contacts has been configured.")) - except Exception as e: - frappe.throw(e) - - -def get_authentication_url(client_id=None, redirect_uri=None): - return { - "url": "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&response_type=code&prompt=consent&client_id={}&include_granted_scopes=true&scope={}&redirect_uri={}".format( - client_id, SCOPES, redirect_uri + if not oauth_code or reauthorize: + return oauth_obj.get_authentication_url( + { + "method": "frappe.integrations.doctype.google_contacts.google_contacts.authorize_access", + "g_contact": g_contact, + "redirect": f"/app/Form/{quote('Google Contacts')}/{quote(g_contact)}", + }, ) - } - -@frappe.whitelist() -def google_callback(code=None): - """ - Authorization code is sent to callback as per the API configuration - """ - google_contact = frappe.cache().hget("google_contacts", "google_contact") - frappe.db.set_value("Google Contacts", google_contact, "authorization_code", code) - frappe.db.commit() - - authorize_access(google_contact) + r = oauth_obj.authorize(oauth_code) + frappe.db.set_value( + "Google Contacts", + g_contact, + {"authorization_code": oauth_code, "refresh_token": r.get("refresh_token")}, + ) def get_google_contacts_object(g_contact): """ Returns an object of Google Calendar along with Google Calendar doc. """ - google_settings = frappe.get_doc("Google Settings") account = frappe.get_doc("Google Contacts", g_contact) + oauth_obj = GoogleOAuth("contacts") - credentials_dict = { - "token": account.get_access_token(), - "refresh_token": account.get_password(fieldname="refresh_token", raise_exception=False), - "token_uri": get_auth_url(), - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "scopes": "https://www.googleapis.com/auth/contacts", - } - - credentials = google.oauth2.credentials.Credentials(**credentials_dict) - google_contacts = build( - serviceName="people", version="v1", credentials=credentials, static_discovery=False + google_contacts = oauth_obj.get_google_service_object( + account.get_access_token(), + account.get_password(fieldname="indexing_refresh_token", raise_exception=False), ) return google_contacts, account diff --git a/frappe/integrations/doctype/google_drive/google_drive.js b/frappe/integrations/doctype/google_drive/google_drive.js index c314d02e7e..b38c0fb8e6 100644 --- a/frappe/integrations/doctype/google_drive/google_drive.js +++ b/frappe/integrations/doctype/google_drive/google_drive.js @@ -41,15 +41,10 @@ frappe.ui.form.on('Google Drive', { } }, authorize_google_drive_access: function(frm) { - let reauthorize = 0; - if (frm.doc.authorization_code) { - reauthorize = 1; - } - frappe.call({ method: "frappe.integrations.doctype.google_drive.google_drive.authorize_access", args: { - "reauthorize": reauthorize + "reauthorize": frm.doc.authorization_code ? 1 : 0 }, callback: function(r) { if (!r.exc) { diff --git a/frappe/integrations/doctype/google_drive/google_drive.py b/frappe/integrations/doctype/google_drive/google_drive.py index c472cbc741..62100ae7c5 100644 --- a/frappe/integrations/doctype/google_drive/google_drive.py +++ b/frappe/integrations/doctype/google_drive/google_drive.py @@ -4,27 +4,22 @@ import os from urllib.parse import quote -import google.oauth2.credentials -import requests from apiclient.http import MediaFileUpload -from googleapiclient.discovery import build from googleapiclient.errors import HttpError import frappe from frappe import _ -from frappe.integrations.doctype.google_settings.google_settings import get_auth_url +from frappe.integrations.google_oauth import GoogleOAuth from frappe.integrations.offsite_backup_utils import ( get_latest_backup_file, send_email, validate_file_size, ) from frappe.model.document import Document -from frappe.utils import get_backups_path, get_bench_path, get_request_site_address +from frappe.utils import get_backups_path, get_bench_path from frappe.utils.background_jobs import enqueue from frappe.utils.backups import new_backup -SCOPES = "https://www.googleapis.com/auth/drive" - class GoogleDrive(Document): def validate(self): @@ -33,118 +28,58 @@ class GoogleDrive(Document): self.backup_folder_id = "" def get_access_token(self): - google_settings = frappe.get_doc("Google Settings") - - if not google_settings.enable: - frappe.throw(_("Google Integration is disabled.")) - if not self.refresh_token: button_label = frappe.bold(_("Allow Google Drive Access")) raise frappe.ValidationError(_("Click on {0} to generate Refresh Token.").format(button_label)) - data = { - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "refresh_token": self.get_password(fieldname="refresh_token", raise_exception=False), - "grant_type": "refresh_token", - "scope": SCOPES, - } - - try: - r = requests.post(get_auth_url(), data=data).json() - except requests.exceptions.HTTPError: - button_label = frappe.bold(_("Allow Google Drive Access")) - frappe.throw( - _( - "Something went wrong during the token generation. Click on {0} to generate a new one." - ).format(button_label) - ) + oauth_obj = GoogleOAuth("drive") + r = oauth_obj.refresh_access_token( + self.get_password(fieldname="refresh_token", raise_exception=False) + ) return r.get("access_token") -@frappe.whitelist() -def authorize_access(reauthorize=None): +@frappe.whitelist(methods=["POST"]) +def authorize_access(reauthorize=False, code=None): """ If no Authorization code get it from Google and then request for Refresh Token. Google Contact Name is set to flags to set_value after Authorization Code is obtained. """ - google_settings = frappe.get_doc("Google Settings") - google_drive = frappe.get_doc("Google Drive") - - redirect_uri = ( - get_request_site_address(True) - + "?cmd=frappe.integrations.doctype.google_drive.google_drive.google_callback" + oauth_code = ( + frappe.db.get_value("Google Drive", "Google Drive", "authorization_code") if not code else code ) + oauth_obj = GoogleOAuth("drive") - if not google_drive.authorization_code or reauthorize: + if not oauth_code or reauthorize: if reauthorize: frappe.db.set_value("Google Drive", None, "backup_folder_id", "") - return get_authentication_url(client_id=google_settings.client_id, redirect_uri=redirect_uri) - else: - try: - data = { - "code": google_drive.authorization_code, - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password( - fieldname="client_secret", raise_exception=False - ), - "redirect_uri": redirect_uri, - "grant_type": "authorization_code", - } - r = requests.post(get_auth_url(), data=data).json() - - if "refresh_token" in r: - frappe.db.set_value("Google Drive", google_drive.name, "refresh_token", r.get("refresh_token")) - frappe.db.commit() - - frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = "/app/Form/{}".format(quote("Google Drive")) - - frappe.msgprint(_("Google Drive has been configured.")) - except Exception as e: - frappe.throw(e) - - -def get_authentication_url(client_id, redirect_uri): - return { - "url": "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&response_type=code&prompt=consent&client_id={}&include_granted_scopes=true&scope={}&redirect_uri={}".format( - client_id, SCOPES, redirect_uri + return oauth_obj.get_authentication_url( + { + "method": "frappe.integrations.doctype.google_drive.google_drive.authorize_access", + "redirect": f"/app/Form/{quote('Google Drive')}", + }, ) - } - -@frappe.whitelist() -def google_callback(code=None): - """ - Authorization code is sent to callback as per the API configuration - """ - frappe.db.set_value("Google Drive", None, "authorization_code", code) - frappe.db.commit() - - authorize_access() + r = oauth_obj.authorize(oauth_code) + frappe.db.set_value( + "Google Drive", + "Google Drive", + {"authorization_code": oauth_code, "refresh_token": r.get("refresh_token")}, + ) def get_google_drive_object(): """ Returns an object of Google Drive. """ - google_settings = frappe.get_doc("Google Settings") account = frappe.get_doc("Google Drive") + oauth_obj = GoogleOAuth("drive") - credentials_dict = { - "token": account.get_access_token(), - "refresh_token": account.get_password(fieldname="refresh_token", raise_exception=False), - "token_uri": get_auth_url(), - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "scopes": "https://www.googleapis.com/auth/drive/v3", - } - - credentials = google.oauth2.credentials.Credentials(**credentials_dict) - google_drive = build( - serviceName="drive", version="v3", credentials=credentials, static_discovery=False + google_drive = oauth_obj.get_google_service_object( + account.get_access_token(), + account.get_password(fieldname="indexing_refresh_token", raise_exception=False), ) return google_drive, account diff --git a/frappe/integrations/doctype/google_settings/google_settings.py b/frappe/integrations/doctype/google_settings/google_settings.py index c70a4b531f..e464e0d090 100644 --- a/frappe/integrations/doctype/google_settings/google_settings.py +++ b/frappe/integrations/doctype/google_settings/google_settings.py @@ -9,10 +9,6 @@ class GoogleSettings(Document): pass -def get_auth_url(): - return "https://www.googleapis.com/oauth2/v4/token" - - @frappe.whitelist() def get_file_picker_settings(): """Return all the data FileUploader needs to start the Google Drive Picker.""" diff --git a/frappe/integrations/doctype/webhook/__init__.py b/frappe/integrations/doctype/webhook/__init__.py index 8e9c72c1a9..192cd2fa12 100644 --- a/frappe/integrations/doctype/webhook/__init__.py +++ b/frappe/integrations/doctype/webhook/__init__.py @@ -17,6 +17,7 @@ def run_webhooks(doc, method): if frappe.flags.webhooks_executed is None: frappe.flags.webhooks_executed = {} + # TODO: remove this hazardous unnecessary cache in flags if frappe.flags.webhooks is None: # load webhooks from cache webhooks = frappe.cache().get_value("webhooks") diff --git a/frappe/integrations/doctype/webhook/test_webhook.py b/frappe/integrations/doctype/webhook/test_webhook.py index 5ac75f5b52..7d9d05cd9e 100644 --- a/frappe/integrations/doctype/webhook/test_webhook.py +++ b/frappe/integrations/doctype/webhook/test_webhook.py @@ -1,6 +1,8 @@ # Copyright (c) 2017, Frappe Technologies and Contributors # License: MIT. See LICENSE +import json import unittest +from contextlib import contextmanager import frappe from frappe.integrations.doctype.webhook.webhook import ( @@ -10,6 +12,16 @@ from frappe.integrations.doctype.webhook.webhook import ( ) +@contextmanager +def get_test_webhook(config): + wh = frappe.get_doc(config).insert() + wh.reload() + try: + yield wh + finally: + wh.delete() + + class TestWebhook(unittest.TestCase): @classmethod def setUpClass(cls): @@ -165,3 +177,31 @@ class TestWebhook(unittest.TestCase): enqueue_webhook(user, webhook) self.assertTrue(frappe.db.get_all("Webhook Request Log", pluck="name")) + + def test_webhook_with_array_body(self): + """Check if array request body are supported.""" + wh_config = { + "doctype": "Webhook", + "webhook_doctype": "Note", + "webhook_docevent": "after_insert", + "enabled": 1, + "request_url": "https://httpbin.org/post", + "request_method": "POST", + "request_structure": "JSON", + "webhook_json": '[\r\n{% for n in range(3) %}\r\n {\r\n "title": "{{ doc.title }}",\r\n "n": {{ n }}\r\n }\r\n {%- if not loop.last -%}\r\n , \r\n {%endif%}\r\n{%endfor%}\r\n]', + "meets_condition": "Yes", + "webhook_headers": [ + { + "key": "Content-Type", + "value": "application/json", + } + ], + } + + with get_test_webhook(wh_config) as wh: + doc = frappe.new_doc("Note") + doc.title = "Test Webhook Note" + + enqueue_webhook(doc, wh) + log = frappe.get_last_doc("Webhook Request Log") + self.assertEqual(len(json.loads(log.response)["json"]), 3) diff --git a/frappe/integrations/doctype/webhook/webhook.js b/frappe/integrations/doctype/webhook/webhook.js index 0953e60625..f4cb4373ea 100644 --- a/frappe/integrations/doctype/webhook/webhook.js +++ b/frappe/integrations/doctype/webhook/webhook.js @@ -72,6 +72,17 @@ frappe.ui.form.on('Webhook', { enable_security: (frm) => { frm.toggle_reqd('webhook_secret', frm.doc.enable_security); + }, + + preview_document: (frm) => { + frappe.call({ + method: "generate_preview", + doc: frm.doc, + callback: (r) => { + frm.refresh_field("meets_condition"); + frm.refresh_field("preview_request_body"); + }, + }); } }); diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json index 880874cb25..a21e460659 100644 --- a/frappe/integrations/doctype/webhook/webhook.json +++ b/frappe/integrations/doctype/webhook/webhook.json @@ -28,7 +28,13 @@ "webhook_headers", "sb_webhook_data", "webhook_data", - "webhook_json" + "webhook_json", + "preview_tab", + "preview_document", + "column_break_26", + "meets_condition", + "section_break_28", + "preview_request_body" ], "fields": [ { @@ -163,13 +169,45 @@ "label": "Request Method", "options": "POST\nPUT\nDELETE", "reqd": 1 + }, + { + "fieldname": "preview_tab", + "fieldtype": "Tab Break", + "label": "Preview" + }, + { + "fieldname": "preview_document", + "fieldtype": "Dynamic Link", + "label": "Select Document", + "options": "webhook_doctype" + }, + { + "fieldname": "preview_request_body", + "fieldtype": "Code", + "is_virtual": 1, + "label": "Request Body" + }, + { + "fieldname": "meets_condition", + "fieldtype": "Data", + "is_virtual": 1, + "label": "Meets Condition?" + }, + { + "fieldname": "column_break_26", + "fieldtype": "Column Break" + }, + { + "fieldname": "section_break_28", + "fieldtype": "Section Break" } ], "links": [], - "modified": "2021-05-25 11:11:28.555291", + "modified": "2022-07-11 08:54:10.740512", "modified_by": "Administrator", "module": "Integrations", "name": "Webhook", + "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ { @@ -187,6 +225,7 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "webhook_doctype", "track_changes": 1 } \ No newline at end of file diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 27fdd662ed..64a98a61e1 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -2,7 +2,6 @@ # License: MIT. See LICENSE import base64 -import datetime import hashlib import hmac import json @@ -27,6 +26,7 @@ class Webhook(Document): self.validate_request_url() self.validate_request_body() self.validate_repeating_fields() + self.preview_document = None def on_update(self): frappe.cache().delete_value("webhooks") @@ -47,7 +47,7 @@ class Webhook(Document): try: frappe.safe_eval(self.condition, eval_locals=get_context(temp_doc)) except Exception as e: - frappe.throw(_(e)) + frappe.throw(_("Invalid Condition: {}").format(e)) def validate_request_url(self): try: @@ -74,6 +74,38 @@ class Webhook(Document): if len(webhook_data) != len(set(webhook_data)): frappe.throw(_("Same Field is entered more than once")) + @frappe.whitelist() + def generate_preview(self): + # This function doesn't need to do anything specific as virtual fields + # get evaluated automatically. + pass + + @property + def meets_condition(self): + if not self.condition: + return _("Yes") + + if not (self.preview_document and self.webhook_doctype): + return _("Select a document to check if it meets conditions.") + + try: + doc = frappe.get_cached_doc(self.webhook_doctype, self.preview_document) + met_condition = frappe.safe_eval(self.condition, eval_locals=get_context(doc)) + except Exception as e: + return _("Failed to evaluate conditions: {}").format(e) + return _("Yes") if met_condition else _("No") + + @property + def preview_request_body(self): + if not (self.preview_document and self.webhook_doctype): + return _("Select a document to preview request data") + + try: + doc = frappe.get_cached_doc(self.webhook_doctype, self.preview_document) + return frappe.as_json(get_webhook_data(doc, self)) + except Exception as e: + return _("Failed to compute request body: {}").format(e) + def get_context(doc): return {"doc": doc, "utils": get_safe_globals().get("frappe").get("utils")} @@ -118,9 +150,9 @@ def log_request(url: str, headers: dict, data: dict, res: requests.Response | No "doctype": "Webhook Request Log", "user": frappe.session.user if frappe.session.user else None, "url": url, - "headers": json.dumps(headers, indent=4) if headers else None, - "data": json.dumps(data, indent=4) if isinstance(data, dict) else data, - "response": json.dumps(res.json(), indent=4) if res else None, + "headers": frappe.as_json(headers) if headers else None, + "data": frappe.as_json(data) if data else None, + "response": frappe.as_json(res.json()) if res else None, } ) diff --git a/frappe/integrations/google_oauth.py b/frappe/integrations/google_oauth.py new file mode 100644 index 0000000000..edce63493e --- /dev/null +++ b/frappe/integrations/google_oauth.py @@ -0,0 +1,187 @@ +import json + +from google.oauth2.credentials import Credentials +from googleapiclient.discovery import build +from requests import get, post + +import frappe +from frappe.utils import get_request_site_address + +CALLBACK_METHOD = "/api/method/frappe.integrations.google_oauth.callback" +_SCOPES = { + "mail": ("https://mail.google.com/"), + "contacts": ("https://www.googleapis.com/auth/contacts"), + "drive": ("https://www.googleapis.com/auth/drive"), + "indexing": ("https://www.googleapis.com/auth/indexing"), +} +_SERVICES = { + "contacts": ("people", "v1"), + "drive": ("drive", "v3"), + "indexing": ("indexing", "v3"), +} + + +class GoogleAuthenticationError(Exception): + pass + + +class GoogleOAuth: + OAUTH_URL = "https://oauth2.googleapis.com/token" + + def __init__(self, domain: str, validate: bool = True): + self.google_settings = frappe.get_single("Google Settings") + self.domain = domain.lower() + self.scopes = ( + " ".join(_SCOPES[self.domain]) + if isinstance(_SCOPES[self.domain], (list, tuple)) + else _SCOPES[self.domain] + ) + + if validate: + self.validate_google_settings() + + def validate_google_settings(self): + google_settings = "Google Settings" + + if not self.google_settings.enable: + frappe.throw(frappe._("Please enable {} before continuing.").format(google_settings)) + + if not (self.google_settings.client_id and self.google_settings.client_secret): + frappe.throw(frappe._("Please update {} before continuing.").format(google_settings)) + + def authorize(self, oauth_code: str) -> dict[str, str | int]: + """Returns a dict with access and refresh token. + + :param oauth_code: code got back from google upon successful auhtorization + :param site_address: side address from which the request is being made + """ + + data = { + "code": oauth_code, + "client_id": self.google_settings.client_id, + "client_secret": self.google_settings.get_password( + fieldname="client_secret", raise_exception=False + ), + "grant_type": "authorization_code", + "scope": self.scopes, + "redirect_uri": get_request_site_address(True) + CALLBACK_METHOD, + } + + return handle_response( + post(self.OAUTH_URL, data=data).json(), + "Google Oauth Authorization Error", + "Something went wrong during the authorization.", + ) + + def refresh_access_token(self, refresh_token: str) -> dict[str, str | int]: + """Refreshes google access token using refresh token""" + + data = { + "client_id": self.google_settings.client_id, + "client_secret": self.google_settings.get_password( + fieldname="client_secret", raise_exception=False + ), + "refresh_token": refresh_token, + "grant_type": "refresh_token", + "scope": self.scopes, + } + + return handle_response( + post(self.OAUTH_URL, data=data).json(), + "Google Oauth Access Token Refresh Error", + "Something went wrong during the access token generation.", + raise_err=True, + ) + + def get_authentication_url(self, state: dict[str, str]) -> dict[str, str]: + """Returns google authentication url. + + :param site_address: side address from which the request is being made (for redirect back to site) + :param state: [optional] dict of values which you need on callback (for calling methods, redirection back to the form, doc name, etc) + """ + + state = json.dumps(state) + callback_url = get_request_site_address(True) + CALLBACK_METHOD + + return { + "url": "https://accounts.google.com/o/oauth2/v2/auth?" + + "access_type=offline&response_type=code&prompt=consent&include_granted_scopes=true&" + + "client_id={}&scope={}&redirect_uri={}&state={}".format( + self.google_settings.client_id, self.scopes, callback_url, state + ) + } + + def get_google_service_object(self, access_token: str, refresh_token: str): + """Returns google service object""" + + credentials_dict = { + "token": access_token, + "refresh_token": refresh_token, + "token_uri": self.OAUTH_URL, + "client_id": self.google_settings.client_id, + "client_secret": self.google_settings.get_password( + fieldname="client_secret", raise_exception=False + ), + "scopes": self.scopes, + } + + return build( + serviceName=_SERVICES[self.domain][0], + version=_SERVICES[self.domain][1], + credentials=Credentials(**credentials_dict), + static_discovery=False, + ) + + +def handle_response( + response: dict[str, str | int], + error_title: str, + error_message: str, + raise_err: bool = False, +): + if "error" in response: + frappe.log_error( + frappe._(error_title), frappe._(response.get("error_description", error_message)) + ) + + if raise_err: + frappe.throw(frappe._(error_title), GoogleAuthenticationError, frappe._(error_message)) + + return {} + + return response + + +def is_valid_access_token(access_token: str) -> bool: + response = get( + "https://oauth2.googleapis.com/tokeninfo", params={"access_token": access_token} + ).json() + + if "error" in response: + return False + + return True + + +@frappe.whitelist(methods=["GET"]) +def callback(state: str, code: str = None, error: str = None) -> None: + """Common callback for google integrations. + Invokes functions using `frappe.get_attr` and also adds required (keyworded) arguments + along with committing and redirecting us back to frappe site.""" + + state = json.loads(state) + redirect = state.pop("redirect", "/app") + success_query_param = state.pop("success_query_param", "") + failure_query_param = state.pop("failure_query_param", "") + + if not error: + state.update({"code": code}) + frappe.get_attr(state.pop("method"))(**state) + + # GET request, hence using commit to persist changes + frappe.db.commit() # nosemgrep + + redirect = f"{redirect}?{failure_query_param if error else success_query_param}" + + frappe.local.response["type"] = "redirect" + frappe.local.response["location"] = redirect diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index d3e7656d6d..0207571e14 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -539,7 +539,9 @@ class BaseDocument: return d = self.get_valid_dict( - convert_dates_to_str=True, ignore_nulls=self.doctype in DOCTYPES_FOR_DOCTYPE + convert_dates_to_str=True, + ignore_nulls=self.doctype in DOCTYPES_FOR_DOCTYPE, + ignore_virtual=True, ) # don't update name, as case might've been changed diff --git a/frappe/public/js/frappe/desk.js b/frappe/public/js/frappe/desk.js index a8cbe020f3..f1a5d00dfd 100644 --- a/frappe/public/js/frappe/desk.js +++ b/frappe/public/js/frappe/desk.js @@ -251,7 +251,6 @@ frappe.Application = class Application { method: 'frappe.email.doctype.email_account.email_account.set_email_password', args: { "email_account": email_account[i]["email_account"], - "user": user, "password": d.get_value("password") }, callback: function(passed) { diff --git a/frappe/public/js/frappe/form/controls/comment.js b/frappe/public/js/frappe/form/controls/comment.js index b9b2d6a987..4550d7045f 100644 --- a/frappe/public/js/frappe/form/controls/comment.js +++ b/frappe/public/js/frappe/form/controls/comment.js @@ -71,6 +71,7 @@ frappe.ui.form.ControlComment = class ControlComment extends frappe.ui.form.Cont const options = super.get_quill_options(); return Object.assign(options, { theme: 'bubble', + bounds: this.quill_container[0], modules: Object.assign(options.modules, { mention: this.get_mention_options() }) @@ -102,7 +103,7 @@ frappe.ui.form.ControlComment = class ControlComment extends frappe.ui.form.Cont get_toolbar_options() { return [ - ['bold', 'italic', 'underline'], + ['bold', 'italic', 'underline', 'strike'], ['blockquote', 'code-block'], [{ 'direction': "rtl" }], ['link', 'image'], diff --git a/frappe/public/js/frappe/form/controls/datetime.js b/frappe/public/js/frappe/form/controls/datetime.js index 43873b3b1e..a086b1b879 100644 --- a/frappe/public/js/frappe/form/controls/datetime.js +++ b/frappe/public/js/frappe/form/controls/datetime.js @@ -85,6 +85,6 @@ frappe.ui.form.ControlDatetime = class ControlDatetime extends frappe.ui.form.Co if (!value && !this.doc) { value = this.last_value; } - return frappe.datetime.get_datetime_as_string(value); + return !value ? "" : frappe.datetime.get_datetime_as_string(value); } }; diff --git a/frappe/public/js/frappe/form/controls/text_editor.js b/frappe/public/js/frappe/form/controls/text_editor.js index d190e11cea..e4e1fff18a 100644 --- a/frappe/public/js/frappe/form/controls/text_editor.js +++ b/frappe/public/js/frappe/form/controls/text_editor.js @@ -184,7 +184,7 @@ frappe.ui.form.ControlTextEditor = class ControlTextEditor extends frappe.ui.for return [ [{ header: [1, 2, 3, false] }], [{ size: font_sizes }], - ['bold', 'italic', 'underline', 'clean'], + ['bold', 'italic', 'underline', 'strike', 'clean'], [{ 'color': [] }, { 'background': [] }], ['blockquote', 'code-block'], // Adding Direction tool to give the user the ability to change text direction. diff --git a/frappe/public/js/frappe/ui/dialog.js b/frappe/public/js/frappe/ui/dialog.js index 1618db9939..3e2d22ffa2 100644 --- a/frappe/public/js/frappe/ui/dialog.js +++ b/frappe/public/js/frappe/ui/dialog.js @@ -145,7 +145,8 @@ frappe.ui.Dialog = class Dialog extends frappe.ui.FieldGroup { return this.get_primary_btn() .removeClass("hide") .html(label) - .click(function() { + .off('click') + .on('click', function() { me.primary_action_fulfilled = true; // get values and send it // as first parameter to click callback diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index 1bf9ec97d9..eb1d9df08f 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -7,6 +7,7 @@ from frappe.query_builder.terms import ParameterizedFunction, ParameterizedValue from frappe.query_builder.utils import ( Column, DocType, + get_qb_engine, get_query_builder, patch_query_aggregation, patch_query_execute, diff --git a/frappe/query_builder/builder.py b/frappe/query_builder/builder.py index d2fdeab324..c23d76974c 100644 --- a/frappe/query_builder/builder.py +++ b/frappe/query_builder/builder.py @@ -1,3 +1,5 @@ +import typing + from pypika import MySQLQuery, Order, PostgreSQLQuery, terms from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder from pypika.queries import QueryBuilder, Schema, Table @@ -13,6 +15,13 @@ class Base: Schema = Schema Table = Table + # Added dynamic type hints for engine attribute + # which is to be assigned later. + if typing.TYPE_CHECKING: + from frappe.database.query import Engine + + engine: Engine + @staticmethod def functions(name: str, *args, **kwargs) -> Function: return Function(name, *args, **kwargs) diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 9e49756340..824de7fbf5 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -1,8 +1,9 @@ +from enum import Enum + from pypika.functions import * from pypika.terms import Arithmetic, ArithmeticExpression, CustomFunction, Function import frappe -from frappe.database.query import Query from frappe.query_builder.custom import GROUP_CONCAT, MATCH, STRING_AGG, TO_TSVECTOR from frappe.query_builder.utils import ImportMapper, db_type_is @@ -14,6 +15,19 @@ class Concat_ws(Function): super().__init__("CONCAT_WS", *terms, **kwargs) +class Locate(Function): + def __init__(self, *terms, **kwargs): + super().__init__("LOCATE", *terms, **kwargs) + + +class Timestamp(Function): + def __init__(self, term: str, time=None, alias=None): + if time: + super().__init__("TIMESTAMP", term, time, alias=alias) + else: + super().__init__("TIMESTAMP", term, alias=alias) + + GroupConcat = ImportMapper({db_type_is.MARIADB: GROUP_CONCAT, db_type_is.POSTGRES: STRING_AGG}) Match = ImportMapper({db_type_is.MARIADB: MATCH, db_type_is.POSTGRES: TO_TSVECTOR}) @@ -73,14 +87,26 @@ class Cast_(Function): def _aggregate(function, dt, fieldname, filters, **kwargs): return ( - Query() - .build_conditions(dt, filters) + frappe.qb.engine.build_conditions(dt, filters) .select(function(PseudoColumn(fieldname))) .run(**kwargs)[0][0] or 0 ) +class SqlFunctions(Enum): + DayOfYear = "dayofyear" + Extract = "extract" + Locate = "locate" + Count = "count" + Sum = "sum" + Avg = "avg" + Max = "max" + Min = "min" + Abs = "abs" + Timestamp = "timestamp" + + def _max(dt, fieldname, filters=None, **kwargs): return _aggregate(Max, dt, fieldname, filters, **kwargs) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index ad8ff4b0df..f0130ca813 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -45,6 +45,12 @@ def get_query_builder(type_of_db: str) -> Postgres | MariaDB: return picks[db] +def get_qb_engine(): + from frappe.database.query import Engine + + return Engine() + + def get_attr(method_string): modulename = ".".join(method_string.split(".")[:-1]) methodname = method_string.split(".")[-1] diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index c1b2e05266..ad9f59b3cd 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -143,7 +143,9 @@ class TestReportview(unittest.TestCase): ) def test_none_filter(self): - query = frappe.db.query.get_sql("DocType", fields="name", filters={"restrict_to_domain": None}) + query = frappe.qb.engine.get_query( + "DocType", fields="name", filters={"restrict_to_domain": None} + ) sql = str(query).replace("`", "").replace('"', "") condition = "restrict_to_domain IS NULL" self.assertIn(condition, sql) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 949c3e9433..8afcaf07d0 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -1,14 +1,16 @@ import unittest import frappe +from frappe.query_builder import Field +from frappe.query_builder.functions import Abs, Count, Max, Timestamp from frappe.tests.test_query_builder import db_type_is, run_only_if -@run_only_if(db_type_is.MARIADB) class TestQuery(unittest.TestCase): + @run_only_if(db_type_is.MARIADB) def test_multiple_tables_in_filters(self): self.assertEqual( - frappe.db.query.get_sql( + frappe.qb.engine.get_query( "DocType", ["*"], [ @@ -18,3 +20,108 @@ class TestQuery(unittest.TestCase): ).get_sql(), "SELECT * FROM `tabDocType` LEFT JOIN `tabBOM Update Log` ON `tabBOM Update Log`.`parent`=`tabDocType`.`name` WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", ) + + def test_string_fields(self): + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields="name, email", filters={"name": "Administrator"} + ).get_sql(), + frappe.qb.from_("User") + .select(Field("name"), Field("email")) + .where(Field("name") == "Administrator") + .get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields=["name, email"], filters={"name": "Administrator"} + ).get_sql(), + frappe.qb.from_("User") + .select(Field("name"), Field("email")) + .where(Field("name") == "Administrator") + .get_sql(), + ) + + def test_functions_fields(self): + self.assertEqual( + frappe.qb.engine.get_query("User", fields="Count(name)", filters={}).get_sql(), + frappe.qb.from_("User").select(Count(Field("name"))).get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query("User", fields=["Count(name)", "Max(name)"], filters={}).get_sql(), + frappe.qb.from_("User").select(Count(Field("name")), Max(Field("name"))).get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields=["abs(name-email)", "Count(name)"], filters={} + ).get_sql(), + frappe.qb.from_("User") + .select(Abs(Field("name") - Field("email")), Count(Field("name"))) + .get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query("User", fields=[Count("*")], filters={}).get_sql(), + frappe.qb.from_("User").select(Count("*")).get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields="timestamp(creation, modified)", filters={} + ).get_sql(), + frappe.qb.from_("User").select(Timestamp(Field("creation"), Field("modified"))).get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields="Count(name) as count, Max(email) as max_email", filters={} + ).get_sql(), + frappe.qb.from_("User") + .select(Count(Field("name")).as_("count"), Max(Field("email")).as_("max_email")) + .get_sql(), + ) + + def test_qb_fields(self): + user_doctype = frappe.qb.DocType("User") + self.assertEqual( + frappe.qb.engine.get_query( + user_doctype, fields=[user_doctype.name, user_doctype.email], filters={} + ).get_sql(), + frappe.qb.from_(user_doctype).select(user_doctype.name, user_doctype.email).get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query(user_doctype, fields=user_doctype.email, filters={}).get_sql(), + frappe.qb.from_(user_doctype).select(user_doctype.email).get_sql(), + ) + + def test_aliasing(self): + user_doctype = frappe.qb.DocType("User") + self.assertEqual( + frappe.qb.engine.get_query( + user_doctype, fields=["name as owner", "email as id"], filters={} + ).get_sql(), + frappe.qb.from_(user_doctype) + .select(user_doctype.name.as_("owner"), user_doctype.email.as_("id")) + .get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + user_doctype, fields="name as owner, email as id", filters={} + ).get_sql(), + frappe.qb.from_(user_doctype) + .select(user_doctype.name.as_("owner"), user_doctype.email.as_("id")) + .get_sql(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + user_doctype, fields=["Count(name) as count", "email as id"], filters={} + ).get_sql(), + frappe.qb.from_(user_doctype) + .select(user_doctype.email.as_("id"), Count(Field("name")).as_("count")) + .get_sql(), + ) diff --git a/frappe/tests/test_query_report.py b/frappe/tests/test_query_report.py index 364cc49e97..ae96898795 100644 --- a/frappe/tests/test_query_report.py +++ b/frappe/tests/test_query_report.py @@ -1,67 +1,72 @@ -# Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors -# License: MIT. See LICENSE - -import unittest - -import frappe -import frappe.utils -from frappe.desk.query_report import build_xlsx_data -from frappe.utils.xlsxutils import make_xlsx - - -class TestQueryReport(unittest.TestCase): - def test_xlsx_data_with_multiple_datatypes(self): - """Test exporting report using rows with multiple datatypes (list, dict)""" - - # Create mock data - data = frappe._dict() - data.columns = [ - {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, - {"label": "Column B", "fieldname": "column_b", "width": 100, "fieldtype": "Float"}, - {"label": "Column C", "fieldname": "column_c", "width": 150, "fieldtype": "Duration"}, - ] - data.result = [ - [1.0, 3.0, 600], - {"column_a": 22.1, "column_b": 21.8, "column_c": 86412}, - {"column_b": 5.1, "column_c": 53234, "column_a": 11.1}, - [3.0, 1.5, 333], - ] - - # Define the visible rows - visible_idx = [0, 2, 3] - - # Build the result - xlsx_data, column_widths = build_xlsx_data(data, visible_idx, include_indentation=0) - - self.assertEqual(type(xlsx_data), list) - self.assertEqual(len(xlsx_data), 4) # columns + data - # column widths are divided by 10 to match the scale that is supported by openpyxl - self.assertListEqual(column_widths, [0, 10, 15]) - - for row in xlsx_data: - self.assertEqual(type(row), list) - - def test_xlsx_export_with_composite_cell_value(self): - """Test excel export using rows with composite cell value""" - - data = frappe._dict() - data.columns = [ - {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, - {"label": "Column B", "fieldname": "column_b", "width": 150, "fieldtype": "Data"}, - ] - data.result = [ - [1.0, "Dummy 1"], - {"column_a": 22.1, "column_b": ["Dummy 1", "Dummy 2"]}, # composite value in column_b - ] - - # Define the visible rows - visible_idx = [0, 1] - - # Build the result - xlsx_data, column_widths = build_xlsx_data(data, visible_idx, include_indentation=0) - # Export to excel - make_xlsx(xlsx_data, "Query Report", column_widths=column_widths) - - for row in xlsx_data: - # column_b should be 'str' even with composite cell value - self.assertEqual(type(row[1]), str) +# Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors +# License: MIT. See LICENSE + +import unittest + +import frappe +import frappe.utils +from frappe.desk.query_report import build_xlsx_data +from frappe.utils.xlsxutils import make_xlsx + + +class TestQueryReport(unittest.TestCase): + def test_xlsx_data_with_multiple_datatypes(self): + """Test exporting report using rows with multiple datatypes (list, dict)""" + + # Create mock data + data = frappe._dict() + data.columns = [ + {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, + {"label": "Column B", "fieldname": "column_b", "width": 100, "fieldtype": "Float"}, + {"label": "Column C", "fieldname": "column_c", "width": 150, "fieldtype": "Duration"}, + ] + data.result = [ + [1.0, 3.0, 600], + {"column_a": 22.1, "column_b": 21.8, "column_c": 86412}, + {"column_b": 5.1, "column_c": 53234, "column_a": 11.1}, + [3.0, 1.5, 333], + ] + + # Define the visible rows + visible_idx = [0, 2, 3] + + # Build the result + xlsx_data, column_widths = build_xlsx_data(data, visible_idx, include_indentation=0) + + self.assertEqual(type(xlsx_data), list) + self.assertEqual(len(xlsx_data), 4) # columns + data + # column widths are divided by 10 to match the scale that is supported by openpyxl + self.assertListEqual(column_widths, [0, 10, 15]) + + for row in xlsx_data: + self.assertIsInstance(row, list) + + # ensure all types are preserved + for row in xlsx_data[1:]: + for cell in row: + self.assertIsInstance(cell, (int, float)) + + def test_xlsx_export_with_composite_cell_value(self): + """Test excel export using rows with composite cell value""" + + data = frappe._dict() + data.columns = [ + {"label": "Column A", "fieldname": "column_a", "fieldtype": "Float"}, + {"label": "Column B", "fieldname": "column_b", "width": 150, "fieldtype": "Data"}, + ] + data.result = [ + [1.0, "Dummy 1"], + {"column_a": 22.1, "column_b": ["Dummy 1", "Dummy 2"]}, # composite value in column_b + ] + + # Define the visible rows + visible_idx = [0, 1] + + # Build the result + xlsx_data, column_widths = build_xlsx_data(data, visible_idx, include_indentation=0) + # Export to excel + make_xlsx(xlsx_data, "Query Report", column_widths=column_widths) + + for row in xlsx_data: + # column_b should be 'str' even with composite cell value + self.assertEqual(type(row[1]), str) diff --git a/frappe/twofactor.py b/frappe/twofactor.py index fc48752193..807032c403 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -479,33 +479,35 @@ def disable(): @frappe.whitelist() def reset_otp_secret(user): + if frappe.session.user != user: + frappe.only_for("System Manager", message=True) + otp_issuer = frappe.db.get_value("System Settings", "System Settings", "otp_issuer_name") user_email = frappe.db.get_value("User", user, "email") - if frappe.session.user in ["Administrator", user]: - clear_default(user + "_otplogin") - clear_default(user + "_otpsecret") - email_args = { - "recipients": user_email, - "sender": None, - "subject": _("OTP Secret Reset - {0}").format(otp_issuer or "Frappe Framework"), - "message": _( - "

Your OTP secret on {0} has been reset. If you did not perform this reset and did not request it, please contact your System Administrator immediately.

" - ).format(otp_issuer or "Frappe Framework"), - "delayed": False, - "retry": 3, - } - enqueue( - method=frappe.sendmail, - queue="short", - timeout=300, - event=None, - is_async=True, - job_name=None, - now=False, - **email_args, - ) - return frappe.msgprint( - _("OTP Secret has been reset. Re-registration will be required on next login.") - ) - else: - return frappe.throw(_("OTP secret can only be reset by the Administrator.")) + + clear_default(user + "_otplogin") + clear_default(user + "_otpsecret") + + email_args = { + "recipients": user_email, + "sender": None, + "subject": _("OTP Secret Reset - {0}").format(otp_issuer or "Frappe Framework"), + "message": _( + "

Your OTP secret on {0} has been reset. If you did not perform this reset and did not request it, please contact your System Administrator immediately.

" + ).format(otp_issuer or "Frappe Framework"), + "delayed": False, + "retry": 3, + } + + enqueue( + method=frappe.sendmail, + queue="short", + timeout=300, + event=None, + is_async=True, + job_name=None, + now=False, + **email_args, + ) + + frappe.msgprint(_("OTP Secret has been reset. Re-registration will be required on next login.")) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 7a1f05220c..81ca143682 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -2114,3 +2114,12 @@ def parse_timedelta(s: str) -> datetime.timedelta: m = TIMEDELTA_BASE_PATTERN.match(s) return datetime.timedelta(**{key: float(val) for key, val in m.groupdict().items()}) + + +def get_job_name(key: str, doctype: str = None, doc_name: str = None) -> str: + job_name = key + if doctype: + job_name += f"_{doctype}" + if doc_name: + job_name += f"_{doc_name}" + return job_name diff --git a/frappe/utils/goal.py b/frappe/utils/goal.py index fe8c2399a4..13c4633031 100644 --- a/frappe/utils/goal.py +++ b/frappe/utils/goal.py @@ -24,7 +24,7 @@ def get_monthly_results( date_format = "%m-%Y" if frappe.db.db_type != "postgres" else "MM-YYYY" return dict( - frappe.db.query.build_conditions(table=goal_doctype, filters=filters) + frappe.qb.engine.build_conditions(table=goal_doctype, filters=filters) .select( DateFormat(Table[date_col], date_format).as_("month_year"), Function(aggregation, goal_field), diff --git a/frappe/website/doctype/website_settings/google_indexing.py b/frappe/website/doctype/website_settings/google_indexing.py index d0657a1928..4f67949f86 100644 --- a/frappe/website/doctype/website_settings/google_indexing.py +++ b/frappe/website/doctype/website_settings/google_indexing.py @@ -4,99 +4,51 @@ from urllib.parse import quote -import google.oauth2.credentials -import requests -from googleapiclient.discovery import build from googleapiclient.errors import HttpError import frappe from frappe import _ -from frappe.integrations.doctype.google_settings.google_settings import get_auth_url -from frappe.utils import get_request_site_address - -SCOPES = "https://www.googleapis.com/auth/indexing" +from frappe.integrations.google_oauth import GoogleOAuth -@frappe.whitelist() -def authorize_access(reauthorize=None): +@frappe.whitelist(methods=["POST"]) +def authorize_access(reauthorize=False, code=None): """If no Authorization code get it from Google and then request for Refresh Token.""" - google_settings = frappe.get_doc("Google Settings") - website_settings = frappe.get_doc("Website Settings") - - redirect_uri = ( - get_request_site_address(True) - + "?cmd=frappe.website.doctype.website_settings.google_indexing.google_callback" + oauth_code = ( + frappe.db.get_value("Website Settings", "Website Settings", "indexing_authorization_code") + if not code + else code ) - if not website_settings.indexing_authorization_code or reauthorize: - return get_authentication_url(client_id=google_settings.client_id, redirect_uri=redirect_uri) - else: - try: - data = { - "code": website_settings.indexing_authorization_code, - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password( - fieldname="client_secret", raise_exception=False - ), - "redirect_uri": redirect_uri, - "grant_type": "authorization_code", - } - res = requests.post(get_auth_url(), data=data).json() + oauth_obj = GoogleOAuth("indexing") - if "refresh_token" in res: - frappe.db.set_value( - "Website Settings", website_settings.name, "indexing_refresh_token", res.get("refresh_token") - ) - frappe.db.commit() - - frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = "/app/Form/{}".format(quote("Website Settings")) - - frappe.msgprint(_("Google Indexing has been configured.")) - except Exception as e: - frappe.throw(e) - - -def get_authentication_url(client_id, redirect_uri): - """Return authentication url with the client id and redirect uri.""" - return { - "url": "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&response_type=code&prompt=consent&client_id={}&include_granted_scopes=true&scope={}&redirect_uri={}".format( - client_id, SCOPES, redirect_uri + if not oauth_code or reauthorize: + return oauth_obj.get_authentication_url( + { + "method": "frappe.website.doctype.website_settings.google_indexing.authorize_access", + "redirect": f"/app/Form/{quote('Website Settings')}", + }, ) - } - -@frappe.whitelist() -def google_callback(code=None): - """Authorization code is sent to callback as per the API configuration.""" - frappe.db.set_value("Website Settings", None, "indexing_authorization_code", code) - frappe.db.commit() - - authorize_access() + res = oauth_obj.authorize(oauth_code) + frappe.db.set_value( + "Website Settings", + "Website Settings", + {"indexing_authorization_code": oauth_code, "indexing_refresh_token": res.get("refresh_token")}, + ) def get_google_indexing_object(): """Returns an object of Google Indexing object.""" - google_settings = frappe.get_doc("Google Settings") account = frappe.get_doc("Website Settings") + oauth_obj = GoogleOAuth("indexing") - credentials_dict = { - "token": account.get_access_token(), - "refresh_token": account.get_password(fieldname="indexing_refresh_token", raise_exception=False), - "token_uri": get_auth_url(), - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "scopes": "https://www.googleapis.com/auth/indexing", - } - - credentials = google.oauth2.credentials.Credentials(**credentials_dict) - google_indexing = build( - serviceName="indexing", version="v3", credentials=credentials, static_discovery=False + return oauth_obj.get_google_service_object( + account.get_access_token(), + account.get_password(fieldname="indexing_refresh_token", raise_exception=False), ) - return google_indexing - def publish_site(url, operation_type="URL_UPDATED"): """Send an update/remove url request.""" diff --git a/frappe/website/doctype/website_settings/website_settings.js b/frappe/website/doctype/website_settings/website_settings.js index 2f15b4c00e..17ff4bed6e 100644 --- a/frappe/website/doctype/website_settings/website_settings.js +++ b/frappe/website/doctype/website_settings/website_settings.js @@ -42,16 +42,10 @@ frappe.ui.form.on('Website Settings', { }, authorize_api_indexing_access: function(frm) { - let reauthorize = 0; - if (frm.doc.authorization_code) { - reauthorize = 1; - } - frappe.call({ method: "frappe.website.doctype.website_settings.google_indexing.authorize_access", args: { - "g_indexing": frm.doc.name, - "reauthorize": reauthorize + "reauthorize": frm.doc.indexing_authorization_code ? 1 : 0 }, callback: function(r) { if (!r.exc) { diff --git a/frappe/website/doctype/website_settings/website_settings.py b/frappe/website/doctype/website_settings/website_settings.py index 4088be88c2..bd634b4f32 100644 --- a/frappe/website/doctype/website_settings/website_settings.py +++ b/frappe/website/doctype/website_settings/website_settings.py @@ -4,12 +4,10 @@ from urllib.parse import quote import frappe from frappe import _ -from frappe.integrations.doctype.google_settings.google_settings import get_auth_url +from frappe.integrations.google_oauth import GoogleOAuth from frappe.model.document import Document from frappe.utils import encode, get_request_site_address -INDEXING_SCOPES = "https://www.googleapis.com/auth/indexing" - class WebsiteSettings(Document): def validate(self): @@ -89,34 +87,14 @@ class WebsiteSettings(Document): frappe.clear_cache() def get_access_token(self): - import requests - - google_settings = frappe.get_doc("Google Settings") - - if not google_settings.enable: - frappe.throw(_("Google Integration is disabled.")) - if not self.indexing_refresh_token: button_label = frappe.bold(_("Allow API Indexing Access")) raise frappe.ValidationError(_("Click on {0} to generate Refresh Token.").format(button_label)) - data = { - "client_id": google_settings.client_id, - "client_secret": google_settings.get_password(fieldname="client_secret", raise_exception=False), - "refresh_token": self.get_password(fieldname="indexing_refresh_token", raise_exception=False), - "grant_type": "refresh_token", - "scope": INDEXING_SCOPES, - } - - try: - res = requests.post(get_auth_url(), data=data).json() - except requests.exceptions.HTTPError: - button_label = frappe.bold(_("Allow Google Indexing Access")) - frappe.throw( - _( - "Something went wrong during the token generation. Click on {0} to generate a new one." - ).format(button_label) - ) + oauth_obj = GoogleOAuth("indexing") + res = oauth_obj.refresh_access_token( + self.get_password(fieldname="indexing_refresh_token", raise_exception=False) + ) return res.get("access_token")