Merge remote-tracking branch 'upstream/develop' into encrypt-otp-secrets

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');
 	});

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();

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) => {

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');
 	});
-});
+});

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):

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")

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()

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');
 

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))
 

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": []
 }

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)

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)

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)

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)

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 = [

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)

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)

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 += ` <a target="_blank" href="https://docs.erpnext.com/docs/v13/user/manual/en/setting-up/email/email_account_setup_with_gmail">${cta}</a>`;
 		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);

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
-}
+}

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:

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
 }

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
 }

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": "<!--email_open_check-->",
 			"unsubscribe_url": "<!--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()

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": []
 }

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) {

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"""

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"];
 		}
 	}
 };

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)
 			)

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,
+	)

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

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,
 		)

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="<h1>" + uni_chr1 + "abcd" + uni_chr2 + "</h1>",
+			message=f"<h1>{uni_chr1}abcd{uni_chr2}</h1>",
 			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"<h1>=EA=80=80abcd=DE=B4</h1>" 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="<h1>\n this is a test of newlines\n" + "</h1>",
 			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"))
 

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
 

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]

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",

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",

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) {

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

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) {

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

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."""

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")

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)

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");
+			},
+		});
 	}
 });
 

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
 }

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,
 		}
 	)
 

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 = "<a href='/app/google-settings'>Google Settings</a>"
+
+		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

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

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) {

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'],

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);
 	}
 };

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.

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

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,

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)

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)
 

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]

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)

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(),
+		)

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)

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": _(
-				"<p>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.</p>"
-			).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": _(
+			"<p>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.</p>"
+		).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."))

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

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),

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."""

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) {

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")