.git-blame-ignore-revs

@@ -17,3 +17,6 @@ fe20515c23a3ac41f1092bf0eaf0a0a452ec2e85
 
 # frappe.cache() -> frappe.cache
 64a732d78fb5fa255bfe5cf648677b74b05982fa
+
+# Bulk refactor with sourcery
+ac7a1b9278a74bd0bf67a399dea554e16b9deabd

.gitlab-ci.yml

-workflow:
-  rules:
-    # Exclude commits with a title containing "[no-ci]"
-    - if: $CI_COMMIT_TITLE =~ /\[no-ci\]/
-      when: never
-
-    # Don't run for the source branch if there is an open merge request (to avoid duplicate builds)
-    # https://docs.gitlab.com/ee/ci/yaml/workflow.html#switch-between-branch-pipelines-and-merge-request-pipelines
-    - if: $CI_COMMIT_BRANCH && $CI_OPEN_MERGE_REQUESTS
-      when: never
-
-    # Don't run for forks, unless either triggered by a maintainer, or the MR is approved
-    - if: ($CI_MERGE_REQUEST_SOURCE_PROJECT_ID && $CI_MERGE_REQUEST_SOURCE_PROJECT_ID != $CI_PROJECT_ID) && ($GITLAB_USER_EMAIL !~ /@dokos\.io$/ && $CI_MERGE_REQUEST_APPROVED != "true")
-      when: never
-
-    # Don't run when MR is tagged "Skip CI"
-    - if: $CI_MERGE_REQUEST_LABELS =~ /Skip CI/
-      when: never
-
-    # Run for merge requests, or...
-    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
-
-    # or run if the branch is the v3.x.x, v3.x.x-hotfix, main, master or develop branch
-    - if: $CI_COMMIT_REF_NAME =~ /(\.x\.x|main|master|develop)/
-
-    # or run if there is a tag
-    - if: $CI_COMMIT_TAG != null
-
-    # or if branch name starts with "ci-"
-    - if: $CI_COMMIT_REF_NAME =~ /^ci-/
+include:
+  - project: "dokos/docli"
+    ref: develop
+    file: "/.gitlab-ci.base.yml"
 
 stages:
   - Unit Tests
@@ -130,26 +104,9 @@ build_docker:
     - echo "Pushing image $IMAGE_NAME"
     - docker push $IMAGE_NAME
 
-    # Push latest tag if the commit is tagged
-    - if [ -n "$CI_COMMIT_TAG" ]; then
+    # Push 'latest' tag if the image tag is a version
+    - if echo "$IMAGE_TAG" | grep -Eq '^v[0-9]+'; then
         N="$CI_REGISTRY_IMAGE:latest";
         echo "Pushing image $N";
-        docker tag $IMAGE_NAME $N;
-        docker push $N;
-      fi
-
-    # Push hotfix tag if the commit is on the hotfix branch
-    - if [ "$CI_COMMIT_REF_NAME" = "v3.x.x-hotfix" ]; then
-        N="$CI_REGISTRY_IMAGE:hotfix";
-        echo "Pushing image $N";
-        docker tag $IMAGE_NAME $N;
-        docker push $N;
-      fi
-
-    # Push develop tag if the commit is on the develop branch
-    - if [ "$CI_COMMIT_REF_NAME" = "develop" ]; then
-        N="$CI_REGISTRY_IMAGE:develop";
-        echo "Pushing image $N";
-        docker tag $IMAGE_NAME $N;
-        docker push $N;
+        docker tag $IMAGE_NAME $N; docker push $N;
       fi

cypress/integration/control_link.js

@@ -152,7 +152,7 @@ context("Control Link", () => {
 
 		cy.get(".frappe-control[data-fieldname=link] input").focus().as("input");
 		cy.wait("@search_link");
-		cy.get("@input").type("todo for link");
+		cy.get("@input").type("todo for link", { delay: 200 });
 		cy.wait("@search_link");
 		cy.get(".frappe-control[data-fieldname=link] ul").should("be.visible");
 		cy.get(".frappe-control[data-fieldname=link] input").type("{enter}", { delay: 100 });
@@ -260,7 +260,7 @@ context("Control Link", () => {
 
 			cy.get(".frappe-control[data-fieldname=link] input").focus().as("input");
 			cy.wait("@search_link");
-			cy.get("@input").type("Sonstiges", { delay: 100 });
+			cy.get("@input").type("Sonstiges", { delay: 200 });
 			cy.wait("@search_link");
 			cy.get(".frappe-control[data-fieldname=link] ul").should("be.visible");
 			cy.get(".frappe-control[data-fieldname=link] input").type("{enter}", { delay: 100 });
@@ -291,7 +291,7 @@ context("Control Link", () => {
 
 		cy.get(".frappe-control[data-fieldname=link] input").focus().as("input");
 		cy.wait("@search_link");
-		cy.get("@input").type("Non-Conforming", { delay: 100 });
+		cy.get("@input").type("Non-Conforming", { delay: 200 });
 		cy.wait("@search_link");
 		cy.get(".frappe-control[data-fieldname=link] ul").should("be.visible");
 		cy.get(".frappe-control[data-fieldname=link] input").type("{enter}", { delay: 100 });

cypress/integration/control_phone.js

@@ -6,6 +6,10 @@ context("Control Phone", () => {
 		cy.visit("/app/website");
 	});
 
+	afterEach(() => {
+		cy.clear_dialogs();
+	});
+
 	function get_dialog_with_phone() {
 		return cy.dialog({
 			title: "Phone",
@@ -20,31 +24,37 @@ context("Control Phone", () => {
 
 	it("should set flag and data", () => {
 		get_dialog_with_phone().as("dialog");
+
 		cy.get(".selected-phone").click();
+		cy.wait(100);
 		cy.get(".phone-picker .phone-wrapper[id='afghanistan']").click();
+		cy.wait(100);
+		cy.get(".selected-phone .country").should("have.text", "+93");
+		cy.get(".selected-phone > img").should("have.attr", "src").and("include", "/af.svg");
+
 		cy.get(".selected-phone").click();
+		cy.wait(100);
 		cy.get(".phone-picker .phone-wrapper[id='india']").click();
+		cy.wait(100);
 		cy.get(".selected-phone .country").should("have.text", "+91");
 		cy.get(".selected-phone > img").should("have.attr", "src").and("include", "/in.svg");
 
 		let phone_number = "9312672712";
 		cy.get(".selected-phone > img").click().first();
-		cy.get_field("phone").first().click({ multiple: true });
+		cy.get_field("phone").first().click();
 		cy.get(".frappe-control[data-fieldname=phone]")
 			.findByRole("textbox")
 			.first()
-			.type(phone_number, { force: true });
+			.type(phone_number);
 
 		cy.get_field("phone").first().should("have.value", phone_number);
-		cy.get_field("phone").first().blur({ force: true });
+		cy.get_field("phone").first().blur();
 		cy.wait(100);
 		cy.get("@dialog").then((dialog) => {
 			let value = dialog.get_value("phone");
 			expect(value).to.equal("+91-" + phone_number);
 		});
-	});
 
-	it("case insensitive search for country and clear search", () => {
 		let search_text = "india";
 		cy.get(".selected-phone").click().first();
 		cy.get(".phone-picker").get(".search-phones").click().type(search_text);

cypress/integration/multi_select_dialog.js

@@ -76,6 +76,11 @@ context("MultiSelectDialog", () => {
 	});
 
 	it("tests more button", () => {
+		cy.get_open_dialog()
+			.get(`.frappe-control[data-fieldname="search_term"]`)
+			.find('input[data-fieldname="search_term"]')
+			.should("exist")
+			.type("Test", { delay: 200 });
 		cy.get_open_dialog()
 			.get(`.frappe-control[data-fieldname="more_child_btn"]`)
 			.should("exist")

cypress/integration/permissions.js

-context("Permissions API", () => {
+context.skip("Permissions API", () => {
 	before(() => {
 		cy.visit("/login");
 		cy.remove_role("frappe@example.com", "System Manager");

frappe/__init__.py

@@ -47,7 +47,7 @@ from .utils.jinja import (
 )
 from .utils.lazy_loader import lazy_import
 
-__version__ = "4.0.0"
+__version__ = "4.1.0"
 __title__ = "Dodock Framework"
 
 controllers = {}
@@ -253,7 +253,7 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False)
 	local.jloader = None
 	local.cache = {}
 	local.form_dict = _dict()
-	local.preload_assets = {"style": [], "script": []}
+	local.preload_assets = {"style": [], "script": [], "icons": []}
 	local.web_translations = {}
 	local.session = _dict()
 	local.dev_server = _dev_server

frappe/automation/doctype/auto_repeat/auto_repeat.py

@@ -154,6 +154,8 @@ class AutoRepeat(Document):
 			else:
 				frappe.throw(_("'Recipients' not specified"))
 
+	# @dokos: No validate_auto_repeat_days
+
 	def update_auto_repeat_id(self):
 		# check if document is already on auto repeat
 		auto_repeat = frappe.db.get_value(self.reference_doctype, self.reference_document, "auto_repeat")

frappe/automation/doctype/milestone/milestone.json

@@ -53,7 +53,7 @@
  ],
  "in_create": 1,
  "links": [],
- "modified": "2022-08-03 15:48:34.261198",
+ "modified": "2023-12-08 15:52:37.525003",
  "modified_by": "Administrator",
  "module": "Automation",
  "name": "Milestone",
@@ -74,7 +74,7 @@
  ],
  "quick_entry": 1,
  "sort_field": "modified",
- "sort_order": "ASC",
+ "sort_order": "DESC",
  "states": [],
  "title_field": "reference_type",
  "track_changes": 1

frappe/automation/doctype/milestone_tracker/milestone_tracker.json

@@ -35,7 +35,7 @@
   }
  ],
  "links": [],
- "modified": "2022-08-03 15:48:33.762630",
+ "modified": "2023-12-08 15:52:37.525003",
  "modified_by": "Administrator",
  "module": "Automation",
  "name": "Milestone Tracker",
@@ -55,7 +55,7 @@
   }
  ],
  "sort_field": "modified",
- "sort_order": "ASC",
+ "sort_order": "DESC",
  "states": [],
  "track_changes": 1
 }

frappe/client.py

@@ -10,6 +10,7 @@ import frappe.utils
 from frappe import _
 from frappe.desk.reportview import validate_args
 from frappe.model.db_query import check_parent_permission
+from frappe.model.utils import is_virtual_doctype
 from frappe.utils import get_safe_filters
 from frappe.utils.deprecations import deprecated
 
@@ -429,6 +430,18 @@ def validate_link(doctype: str, docname: str, fields=None):
 		)
 
 	values = frappe._dict()
+
+	if is_virtual_doctype(doctype):
+		try:
+			frappe.get_doc(doctype, docname)
+			values.name = docname
+		except frappe.DoesNotExistError:
+			frappe.clear_last_message()
+			frappe.msgprint(
+				_("Document {0} {1} does not exist").format(frappe.bold(doctype), frappe.bold(docname)),
+			)
+		return values
+
 	values.name = frappe.db.get_value(doctype, docname, cache=True)
 
 	fields = frappe.parse_json(fields)

frappe/commands/site.py

@@ -526,6 +526,130 @@ def list_apps(context, format):
 		click.echo(frappe.as_json(summary_dict))
 
 
+@click.command("add-database-index")
+@click.option("--doctype", help="DocType on which index needs to be added")
+@click.option(
+	"--column",
+	multiple=True,
+	help="Column to index. Multiple columns will create multi-column index in given order. To create a multiple, single column index, execute the command multiple times.",
+)
+@pass_context
+def add_db_index(context, doctype, column):
+	"Adds a new DB index and creates a property setter to persist it."
+	from frappe.custom.doctype.property_setter.property_setter import make_property_setter
+
+	columns = column  # correct naming
+	for site in context.sites:
+		frappe.connect(site=site)
+		try:
+			frappe.db.add_index(doctype, columns)
+			if len(columns) == 1:
+				make_property_setter(
+					doctype,
+					columns[0],
+					property="search_index",
+					value="1",
+					property_type="Check",
+					for_doctype=False,  # Applied on docfield
+				)
+			frappe.db.commit()
+		finally:
+			frappe.destroy()
+
+	if not context.sites:
+		raise SiteNotSpecifiedError
+
+
+@click.command("describe-database-table")
+@click.option("--doctype", help="DocType to describe")
+@click.option(
+	"--column",
+	multiple=True,
+	help="Explicitly fetch accurate cardinality from table data. This can be quite slow on large tables.",
+)
+@pass_context
+def describe_database_table(context, doctype, column):
+	"""Describes various statistics about the table.
+
+	This is useful to build integration like
+	This includes:
+	1. Schema
+	2. Indexes
+	3. stats - total count of records
+	4. if column is specified then extra stats are generated for column:
+	        Distinct values count in column
+	"""
+	import json
+
+	for site in context.sites:
+		frappe.connect(site=site)
+		try:
+			data = _extract_table_stats(doctype, column)
+			# NOTE: Do not print anything else in this to avoid clobbering the output.
+			print(json.dumps(data, indent=2))
+		finally:
+			frappe.destroy()
+
+	if not context.sites:
+		raise SiteNotSpecifiedError
+
+
+def _extract_table_stats(doctype: str, columns: list[str]) -> dict:
+	from frappe.utils import cstr, get_table_name
+
+	def sql_bool(val):
+		return cstr(val).lower() in ("yes", "1", "true")
+
+	table = get_table_name(doctype, wrap_in_backticks=True)
+
+	schema = []
+	for field in frappe.db.sql(f"describe {table}", as_dict=True):
+		schema.append(
+			{
+				"column": field["Field"],
+				"type": field["Type"],
+				"is_nullable": sql_bool(field["Null"]),
+				"default": field["Default"],
+			}
+		)
+
+	def update_cardinality(column, value):
+		for col in schema:
+			if col["column"] == column:
+				col["cardinality"] = value
+				break
+
+	indexes = []
+	for idx in frappe.db.sql(f"show index from {table}", as_dict=True):
+		indexes.append(
+			{
+				"unique": not sql_bool(idx["Non_unique"]),
+				"cardinality": idx["Cardinality"],
+				"name": idx["Key_name"],
+				"sequence": idx["Seq_in_index"],
+				"nullable": sql_bool(idx["Null"]),
+				"column": idx["Column_name"],
+				"type": idx["Index_type"],
+			}
+		)
+		if idx["Seq_in_index"] == 1:
+			update_cardinality(idx["Column_name"], idx["Cardinality"])
+
+	total_rows = frappe.db.count(doctype)
+
+	# fetch accurate cardinality for columns by query. WARN: This can take a lot of time.
+	for column in columns:
+		cardinality = frappe.db.sql(f"select count(distinct {column}) from {table}")[0][0]
+		update_cardinality(column, cardinality)
+
+	return {
+		"table_name": table.strip("`"),
+		"total_rows": total_rows,
+		"schema": schema,
+		"indexes": indexes,
+	}
+
+
 @click.command("add-system-manager")
 @click.argument("email")
 @click.option("--first-name")
@@ -1275,7 +1399,7 @@ def trim_database(context, dry_run, format, no_backup, yes=False):
 		for table_name in database_tables:
 			if not table_name.startswith("tab"):
 				continue
-			if not (table_name.replace("tab", "", 1) in doctype_tables or table_name in STANDARD_TABLES):
+			if table_name.replace("tab", "", 1) not in doctype_tables and table_name not in STANDARD_TABLES:
 				TABLES_TO_DROP.append(table_name)
 
 		if not TABLES_TO_DROP:
@@ -1422,6 +1546,8 @@ def add_new_user(
 commands = [
 	add_system_manager,
 	add_user_for_sites,
+	add_db_index,
+	describe_database_table,
 	backup,
 	drop_site,
 	install_app,

frappe/contacts/doctype/contact/contact.json

@@ -247,7 +247,7 @@
  "image_field": "image",
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2023-10-12 08:10:03.499154",
+ "modified": "2023-12-08 15:52:37.525003",
  "modified_by": "Administrator",
  "module": "Contacts",
  "name": "Contact",
@@ -283,7 +283,7 @@
  "show_preview_popup": 1,
  "show_title_field_in_link": 1,
  "sort_field": "modified",
- "sort_order": "ASC",
+ "sort_order": "DESC",
  "states": [],
  "title_field": "full_name"
 }

frappe/contacts/doctype/contact/contact.py

@@ -52,14 +52,14 @@ class Contact(Document):
 		# concat first and last name
 		self.name = self._get_full_name()
 
-		if frappe.db.exists("Contact", self.name):
-			self.name = append_number_if_name_exists("Contact", self.name)
-
 		# concat party name if reqd
 		for link in self.links:
 			self.name = self.name + "-" + link.link_name.strip()
 			break
 
+		if frappe.db.exists("Contact", self.name):
+			self.name = append_number_if_name_exists("Contact", self.name)
+
 	def validate(self):
 		self.full_name = self._get_full_name()
 		self.set_primary_email()

frappe/core/doctype/communication/communication.py

@@ -705,7 +705,10 @@ def update_parent_document_on_communication(doc):
 
 def update_first_response_time(parent, communication):
 	if parent.meta.has_field("first_response_time") and not parent.get("first_response_time"):
-		if is_system_user(communication.sender):
+		if (
+			is_system_user(communication.sender)
+			or frappe.get_cached_value("User", frappe.session.user, "user_type") == "System User"
+		):
 			if communication.sent_or_received == "Sent":
 				first_responded_on = communication.creation
 				if parent.meta.has_field("first_responded_on"):

frappe/core/doctype/communication/email.py

@@ -195,7 +195,8 @@ def _make(
 def validate_email(doc: "Communication") -> None:
 	"""Validate Email Addresses of Recipients and CC"""
 	if (
-		not (doc.communication_type == "Communication" and doc.communication_medium == "Email")
+		doc.communication_type != "Communication"
+		or doc.communication_medium != "Email"
 		or doc.flags.in_receive
 	):
 		return

frappe/core/doctype/communication/mixins.py

 import frappe
 from frappe import _
 from frappe.core.utils import get_parent_doc
+from frappe.desk.doctype.notification_settings.notification_settings import (
+	is_email_notifications_enabled_for_type,
+)
 from frappe.desk.doctype.todo.todo import ToDo
 from frappe.email.doctype.email_account.email_account import EmailAccount
 from frappe.utils import get_formatted_email, get_url, parse_addr
@@ -78,7 +81,12 @@ class CommunicationEmailMixin:
 			if doc_owner := self.get_owner():
 				cc.append(doc_owner)
 			cc = set(cc) - {self.sender_mailid}
-			cc.update(self.get_assignees())
+			assignees = set(self.get_assignees())
+			# Check and remove If user disabled notifications for incoming emails on assigned document.
+			for assignee in assignees.copy():
+				if not is_email_notifications_enabled_for_type(assignee, "threads_on_assigned_document"):
+					assignees.remove(assignee)
+			cc.update(assignees)
 
 		cc = set(cc) - set(self.filter_thread_notification_disbled_users(cc))
 		cc = cc - set(self.mail_recipients(is_inbound_mail_communcation=is_inbound_mail_communcation))

frappe/core/doctype/data_import/data_import.js

@@ -136,47 +136,40 @@ frappe.ui.form.on("Data Import", {
 				let total_records = cint(r.message.total_records);
 
 				if (!total_records) return;
+				let action, message;
+				if (frm.doc.import_type === "Insert New Records") {
+					action = "imported";
+				} else {
+					action = "updated";
+				}
 
-				let message;
 				if (failed_records === 0) {
-					let message_args = [successful_records];
-					if (frm.doc.import_type === "Insert New Records") {
-						message =
-							successful_records > 1
-								? __("Successfully imported {0} records.", message_args)
-								: __("Successfully imported {0} record.", message_args);
+					let message_args = [action, successful_records];
+					if (successful_records === 1) {
+						message = __("Successfully {0} 1 record.", message_args);
 					} else {
-						message =
-							successful_records > 1
-								? __("Successfully updated {0} records.", message_args)
-								: __("Successfully updated {0} record.", message_args);
+						message = __("Successfully {0} {1} records.", message_args);
 					}
 				} else {
-					let message_args = [successful_records, total_records];
-					if (frm.doc.import_type === "Insert New Records") {
-						message =
-							successful_records > 1
-								? __(
-										"Successfully imported {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.",
-										message_args
-								  )
-								: __(
-										"Successfully imported {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.",
+					let message_args = [action, successful_records, total_records];
+					if (successful_records === 1) {
+						message = __(
+							"Successfully {0} {1} record out of {2}. Click on Export Errored Rows, fix the errors and import again.",
 							message_args
 						);
 					} else {
-						message =
-							successful_records > 1
-								? __(
-										"Successfully updated {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.",
-										message_args
-								  )
-								: __(
-										"Successfully updated {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.",
+						message = __(
+							"Successfully {0} {1} records out of {2}. Click on Export Errored Rows, fix the errors and import again.",
 							message_args
 						);
 					}
 				}
+
+				// If the job timed out, display an extra hint
+				if (r.message.status === "Timed Out") {
+					message += "<br/>" + __("Import timed out, please re-try.");
+				}
+
 				frm.dashboard.set_headline(message);
 			},
 		});

frappe/core/doctype/data_import/data_import.json

@@ -95,7 +95,7 @@
    "hidden": 1,
    "label": "Status",
    "no_copy": 1,
-      "options": "Pending\nSuccess\nPartial Success\nError",
+   "options": "Pending\nSuccess\nPartial Success\nError\nTimed Out",
    "read_only": 1
   },
   {
@@ -171,7 +171,7 @@
  ],
  "hide_toolbar": 1,
  "links": [],
-  "modified": "2022-02-14 10:08:37.624914",
+ "modified": "2023-12-15 12:45:49.452834",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "Data Import",

frappe/core/doctype/data_import/data_import.py

@@ -3,6 +3,8 @@
 
 import os
 
+from rq.timeouts import JobTimeoutException
+
 import frappe
 from frappe import _
 from frappe.core.doctype.data_import.exporter import Exporter
@@ -32,10 +34,11 @@ class DataImport(Document):
 		payload_count: DF.Int
 		reference_doctype: DF.Link
 		show_failed_logs: DF.Check
-		status: DF.Literal["Pending", "Success", "Partial Success", "Error"]
+		status: DF.Literal["Pending", "Success", "Partial Success", "Error", "Timed Out"]
 		submit_after_import: DF.Check
 		template_options: DF.Code | None
 		template_warnings: DF.Code | None
+
 	# end: auto-generated types
 
 	def validate(self):
@@ -137,6 +140,9 @@ def start_import(data_import):
 	try:
 		i = Importer(data_import.reference_doctype, data_import=data_import)
 		i.import_data()
+	except JobTimeoutException:
+		frappe.db.rollback()
+		data_import.db_set("status", "Timed Out")
 	except Exception:
 		frappe.db.rollback()
 		data_import.db_set("status", "Error")
@@ -191,6 +197,9 @@ def download_import_log(data_import_name):
 def get_import_status(data_import_name):
 	import_status = {}
 
+	data_import = frappe.get_doc("Data Import", data_import_name)
+	import_status["status"] = data_import.status
+
 	logs = frappe.get_all(
 		"Data Import Log",
 		fields=["count(*) as count", "success"],