fix: Add typing to whitelisted methods

press/api/email.py

@@ -79,16 +79,13 @@ def setup(site):
 
 
 @frappe.whitelist(allow_guest=True)
-def get_analytics(**data):
+def get_analytics(month: int, status: str, site: str, key: str):
 	"""
 	send data for a specific month
 	"""
-	month = data.get("month")
 	year = datetime.now().year
 	last_day = calendar.monthrange(year, int(month))[1]
-	status = data.get("status")
-	site = data.get("site")
-	subscription_key = data.get("key")
+	subscription_key = key
 
 	for value in (site, subscription_key):
 		if not value or not isinstance(value, str):
@@ -207,25 +204,25 @@ def check_recipients(recipients: str | list[str]):
 
 
 @frappe.whitelist(allow_guest=True)
-def send_mime_mail(**data):
+def send_mime_mail(data: str):
 	"""
 	send api request to mailgun
 	"""
 	files = frappe._dict(frappe.request.files)
-	data = json.loads(data["data"])
+	data_dict = json.loads(data)
 
-	validate_plan(data["sk_mail"])
+	validate_plan(data_dict["sk_mail"])
 
 	api_key, domain = frappe.db.get_value("Press Settings", None, ["mailgun_api_key", "root_domain"])
 
 	message: bytes = files["mime"].read()
 	check_spam(message)
-	check_recipients(data["recipients"])
+	check_recipients(data_dict["recipients"])
 
 	resp = requests.post(
 		f"https://api.mailgun.net/v3/{domain}/messages.mime",
 		auth=("api", f"{api_key}"),
-		data={"to": data["recipients"], "v:sk_mail": data["sk_mail"]},
+		data={"to": data_dict["recipients"], "v:sk_mail": data_dict["sk_mail"]},
 		files={"message": message},
 	)
 
@@ -234,7 +231,7 @@ def send_mime_mail(**data):
 	if resp.status_code == 400:
 		err_msg: str = resp.json().get("message", "Invalid request")
 		frappe.throw(f"Something went wrong with sending emails: {err_msg}", InvalidEmail)
-	log_error("Email Delivery Service: Sending error", response=resp.text, data=data, message=message)
+	log_error("Email Delivery Service: Sending error", response=resp.text, data=data_dict, message=message)
 	frappe.throw(
 		"Something went wrong with sending emails. Please try again later or raise a support ticket with support.frappe.io",
 		EmailSendError,

press/api/github.py

@@ -302,7 +302,7 @@ def repositories(installation, token):
 
 
 @frappe.whitelist()
-def repository(owner, name, installation=None):
+def repository(owner: str, name: str, installation: str | None = None):
 	token = ""
 	if not installation:
 		token = frappe.db.get_value("Press Settings", "github_access_token")
@@ -337,7 +337,7 @@ def repository(owner, name, installation=None):
 
 
 @frappe.whitelist()
-def app(owner, repository, branch, installation=None):
+def app(owner: str, repository: str, branch: str, installation: str | None = None):
 	headers = get_auth_headers(installation)
 	response = requests.get(
 		f"https://api.github.com/repos/{owner}/{repository}/branches/{branch}",
@@ -381,7 +381,7 @@ def app(owner, repository, branch, installation=None):
 
 
 @frappe.whitelist()
-def branches(owner, name, installation=None, app_source=None):
+def branches(owner: str, name: str, installation: str | None = None, app_source: str | None = None):
 	"""
 	Return ALL branches for the repo, following GitHub pagination.
 	"""
@@ -390,7 +390,7 @@ def branches(owner, name, installation=None, app_source=None):
 
 	headers = get_auth_headers(installation)
 
-	out = []
+	out: list[dict] = []
 	page = 1
 	while True:
 		resp = requests.get(
@@ -487,7 +487,7 @@ def _get_app_name_and_title_from_hooks(
 	branch_info,
 	headers,
 	tree,
-) -> tuple[str, str] | None:
+) -> tuple[str, str]:
 	reason_for_invalidation = f"Files {frappe.bold('hooks.py or patches.txt')} not found."
 	for directory, files in tree.items():
 		if not files:
@@ -524,7 +524,7 @@ def _get_app_name_and_title_from_hooks(
 		break
 
 	frappe.throw(f"Not a valid Frappe App! {reason_for_invalidation}")
-	return None
+	raise  # for mypy: NoReturn
 
 
 def _generate_files_tree(files):

press/api/monitoring.py

@@ -159,7 +159,7 @@ def get_targets_method_rate_limit() -> int:
 
 @frappe.whitelist(allow_guest=True)
 @rate_limit(limit=get_targets_method_rate_limit, seconds=MONITORING_ENDPOINT_RATE_LIMIT_WINDOW_SECONDS)
-def targets(token=None):
+def targets(token: str | None = None):
 	if not token:
 		frappe.throw_permission_error()
 	monitor_token = frappe.db.get_single_value("Press Settings", "monitor_token", cache=True)

press/api/tests/test_email.py

@@ -0,0 +1,124 @@
+# Copyright (c) 2024, Frappe and contributors
+# For license information, please see license.txt
+
+from __future__ import annotations
+
+import io
+import json
+from unittest.mock import Mock, patch
+
+import frappe
+from frappe.tests.test_api import FrappeAPITestCase, make_request
+
+
+class TestSendMimeMail(FrappeAPITestCase):
+	"""Test send_mime_mail endpoint with the same parameters email_delivery_service uses."""
+
+	ENDPOINT = "/api/method/press.api.email.send_mime_mail"
+
+	def _post_mime_mail(
+		self,
+		data: dict,
+		mime_content: bytes = b"MIME-Version: 1.0\r\nContent-Type: text/plain\r\n\r\nTest email body",
+	):
+		"""Send a multipart form request with a MIME file, matching email_delivery_service.
+
+		The email_delivery_service app sends:
+		requests.post(url, data={"data": json.dumps(data)}, files={"mime": msg})
+		"""
+		return make_request(
+			target=self.TEST_CLIENT.post,
+			args=(self.ENDPOINT,),
+			kwargs={
+				"data": {
+					"data": json.dumps(data),
+					"mime": (io.BytesIO(mime_content), "message.eml"),
+				},
+				"content_type": "multipart/form-data",
+				"buffered": True,
+			},
+		)
+
+	@patch("press.api.email.validate_plan")
+	@patch("press.api.email.check_spam")
+	@patch("press.api.email.requests.post")
+	def test_send_mime_mail_success(self, mock_mailgun_post, mock_check_spam, mock_validate_plan):
+		"""Test that send_mime_mail correctly parses the data parameter as a JSON string,
+		using the same request format as email_delivery_service."""
+		mock_mailgun_response = Mock()
+		mock_mailgun_response.status_code = 200
+		mock_mailgun_post.return_value = mock_mailgun_response
+
+		frappe.db.set_single_value("Press Settings", "mailgun_api_key", "test-key")
+		frappe.db.set_single_value("Press Settings", "root_domain", "example.com")
+
+		data = {
+			"sender": "sender@example.com",
+			"recipients": "recipient@example.com",
+			"sk_mail": "test-secret-key",
+			"site": "test.frappe.cloud",
+		}
+		mime_content = b"MIME-Version: 1.0\r\nContent-Type: text/plain\r\n\r\nTest email body"
+
+		response = self._post_mime_mail(data, mime_content)
+
+		self.assertEqual(response.status_code, 200)
+		self.assertEqual(response.json["message"], "Sending")
+
+		mock_validate_plan.assert_called_once_with("test-secret-key")
+		mock_check_spam.assert_called_once_with(mime_content)
+		mock_mailgun_post.assert_called_once()
+
+		call_kwargs = mock_mailgun_post.call_args
+		self.assertEqual(call_kwargs.kwargs["data"]["to"], "recipient@example.com")
+		self.assertEqual(call_kwargs.kwargs["data"]["v:sk_mail"], "test-secret-key")
+
+	@patch("press.api.email.validate_plan")
+	@patch("press.api.email.check_spam", new=Mock())
+	@patch("press.api.email.requests.post")
+	def test_send_mime_mail_parses_json_string_data(self, mock_mailgun_post, mock_validate_plan):
+		"""Verify the data parameter is correctly parsed as a JSON string (not a dict)."""
+		mock_mailgun_response = Mock()
+		mock_mailgun_response.status_code = 200
+		mock_mailgun_post.return_value = mock_mailgun_response
+
+		frappe.db.set_single_value("Press Settings", "mailgun_api_key", "test-key")
+		frappe.db.set_single_value("Press Settings", "root_domain", "example.com")
+
+		data = {
+			"sender": "test@site.com",
+			"recipients": "user@domain.com",
+			"sk_mail": "sk-12345",
+			"site": "mysite.frappe.cloud",
+		}
+
+		response = self._post_mime_mail(data)
+
+		self.assertEqual(response.status_code, 200)
+		# Verify validate_plan received the correct sk_mail from the parsed JSON
+		mock_validate_plan.assert_called_once_with("sk-12345")
+
+	@patch("press.api.email.validate_plan", new=Mock())
+	@patch("press.api.email.check_spam", new=Mock())
+	def test_send_mime_mail_mailgun_400_error(self):
+		"""Test that a 400 response from mailgun raises InvalidEmail."""
+
+		frappe.db.set_single_value("Press Settings", "mailgun_api_key", "test-key")
+		frappe.db.set_single_value("Press Settings", "root_domain", "example.com")
+
+		data = {
+			"sender": "sender@example.com",
+			"recipients": "invalid",
+			"sk_mail": "test-secret-key",
+			"site": "test.frappe.cloud",
+		}
+
+		response = self._post_mime_mail(data)
+
+		self.assertIn(response.status_code, (400, 417))
+
+	def test_email_ping(self):
+		"""Test the email_ping endpoint."""
+		response = self.get(self.method("press.api.email.email_ping"))
+		self.assertEqual(response.status_code, 200)
+		self.assertEqual(response.json["message"], "pong")

press/press/doctype/site/site.py

@@ -1309,7 +1309,7 @@ def ready_for_move(self):
 	def check_fatal_site_update(self):
 		if self.fatal_site_update:
 			frappe.throw(
-				"Site has encountered a fatal error during last update. Please open a ticket on our <a href='https://support.frappe.io'> support portal </a> with the error details to resolve the issue.",
+				"Site has encountered a fatal error during last update. Please open a ticket on our <a href='https://support.frappe.io' class='underline'> support portal </a> with the error details to resolve the issue.",
 			)
 
 	@dashboard_whitelist()

press/press/doctype/site_update/site_update.py

@@ -237,7 +237,7 @@ def validate_apps(self):
 
 	def before_insert(self):
 		self.backup_type = "Logical"
-		site: "Site" = frappe.get_cached_doc("Site", self.site)
+		site: "Site" = frappe.get_doc("Site", self.site)
 		site.check_move_scheduled()
 		site.check_fatal_site_update()
 

press/utils/dns.py

@@ -145,7 +145,7 @@ def accessible_link_substr(provider: str):
 	except requests.RequestException:
 		return None
 	else:
-		return f'<a class=underline href="http://{provider}" target="_blank">{provider}</a>'
+		return f'<a class="underline" href="http://{provider}" target="_blank">{provider}</a>'
 
 
 @redis_cache()