From 27ca321f9565c6fe6d00f439c3409fb53d113a8c Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Thu, 10 Oct 2024 12:16:52 +0530 Subject: [PATCH 1/2] refactor!: convert `Mail Agent` to a non-child DocType --- mail/mail/doctype/mail_agent/mail_agent.js | 8 ++ mail/mail/doctype/mail_agent/mail_agent.json | 114 ++++++++++++++---- mail/mail/doctype/mail_agent/mail_agent.py | 73 ++++++++++- .../doctype/mail_agent/test_mail_agent.py | 9 ++ mail/mail/doctype/mail_domain/mail_domain.py | 6 +- .../doctype/mail_settings/mail_settings.json | 14 +-- .../doctype/mail_settings/mail_settings.py | 42 +------ 7 files changed, 185 insertions(+), 81 deletions(-) create mode 100644 mail/mail/doctype/mail_agent/mail_agent.js create mode 100644 mail/mail/doctype/mail_agent/test_mail_agent.py diff --git a/mail/mail/doctype/mail_agent/mail_agent.js b/mail/mail/doctype/mail_agent/mail_agent.js new file mode 100644 index 00000000..6458dd2f --- /dev/null +++ b/mail/mail/doctype/mail_agent/mail_agent.js @@ -0,0 +1,8 @@ +// Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors +// For license information, please see license.txt + +// frappe.ui.form.on("Mail Agent", { +// refresh(frm) { + +// }, +// }); diff --git a/mail/mail/doctype/mail_agent/mail_agent.json b/mail/mail/doctype/mail_agent/mail_agent.json index 7fe77bb3..e6279c29 100644 --- a/mail/mail/doctype/mail_agent/mail_agent.json +++ b/mail/mail/doctype/mail_agent/mail_agent.json @@ -1,29 +1,70 @@ { "actions": [], - "allow_rename": 1, "creation": "2024-10-07 14:22:45.181308", + "default_view": "List", "doctype": "DocType", - "editable_grid": 1, "engine": "InnoDB", "field_order": [ - "section_break_dhue", + "section_break_excr", + "enabled", + "section_break_whcz", "type", - "column_break_dxky", - "host", - "priority" + "section_break_syxq", + "agent", + "priority", + "section_break_3jtz", + "ipv4", + "column_break_mgw4", + "ipv6" ], "fields": [ { - "fieldname": "section_break_dhue", - "fieldtype": "Section Break" + "fieldname": "agent", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Agent", + "no_copy": 1, + "reqd": 1, + "set_only_once": 1, + "unique": 1 + }, + { + "fieldname": "section_break_syxq", + "fieldtype": "Column Break" + }, + { + "fieldname": "ipv4", + "fieldtype": "Data", + "in_standard_filter": 1, + "label": "IPv4", + "no_copy": 1, + "read_only": 1 }, { - "fieldname": "host", + "fieldname": "ipv6", "fieldtype": "Data", + "in_standard_filter": 1, + "label": "IPv6", + "no_copy": 1, + "read_only": 1 + }, + { + "default": "1", + "depends_on": "eval: !doc.__islocal", + "fieldname": "enabled", + "fieldtype": "Check", + "label": "Enabled", + "search_index": 1 + }, + { + "fieldname": "type", + "fieldtype": "Select", "in_list_view": 1, - "label": "Host", + "label": "Type", + "options": "\nInbound\nOutbound", "reqd": 1, - "unique": 1 + "search_index": 1, + "set_only_once": 1 }, { "depends_on": "eval: doc.type == \"Inbound\"", @@ -32,32 +73,55 @@ "in_list_view": 1, "label": "Priority", "mandatory_depends_on": "eval: doc.type == \"Inbound\"", - "non_negative": 1, - "search_index": 1 + "non_negative": 1 }, { - "fieldname": "type", - "fieldtype": "Select", - "in_list_view": 1, - "label": "Type", - "options": "\nInbound\nOutbound", - "reqd": 1 + "fieldname": "section_break_excr", + "fieldtype": "Section Break" + }, + { + "fieldname": "section_break_whcz", + "fieldtype": "Section Break" }, { - "fieldname": "column_break_dxky", + "fieldname": "section_break_3jtz", + "fieldtype": "Section Break" + }, + { + "fieldname": "column_break_mgw4", "fieldtype": "Column Break" } ], "index_web_pages_for_search": 1, - "istable": 1, "links": [], - "modified": "2024-10-07 17:25:15.659023", + "modified": "2024-10-10 11:04:56.747870", "modified_by": "Administrator", "module": "Mail", "name": "Mail Agent", + "naming_rule": "Set by user", "owner": "Administrator", - "permissions": [], - "sort_field": "creation", + "permissions": [ + { + "create": 1, + "delete": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "write": 1 + }, + { + "role": "Domain Owner", + "select": 1 + }, + { + "role": "Mailbox User", + "select": 1 + } + ], + "sort_field": "modified", "sort_order": "DESC", - "states": [] + "states": [], + "track_changes": 1 } \ No newline at end of file diff --git a/mail/mail/doctype/mail_agent/mail_agent.py b/mail/mail/doctype/mail_agent/mail_agent.py index b0e6e152..82348b91 100644 --- a/mail/mail/doctype/mail_agent/mail_agent.py +++ b/mail/mail/doctype/mail_agent/mail_agent.py @@ -1,9 +1,78 @@ # Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -# import frappe +import frappe +from frappe import _ +from mail.utils import get_dns_record from frappe.model.document import Document +from mail.mail.doctype.dns_record.dns_record import create_or_update_dns_record +from mail.mail.doctype.mail_settings.mail_settings import validate_mail_settings class MailAgent(Document): - pass + def autoname(self) -> None: + self.agent = self.agent.lower() + self.name = self.agent + + def validate(self) -> None: + if self.is_new(): + validate_mail_settings() + + self.validate_agent() + + def on_update(self) -> None: + if self.type == "Outbound": + create_or_update_spf_dns_record() + + def on_trash(self) -> None: + if frappe.session.user != "Administrator": + frappe.throw(_("Only Administrator can delete Mail Agent.")) + + self.db_set("enabled", 0) + create_or_update_spf_dns_record() + + def validate_agent(self) -> None: + """Validates the agent and fetches the IP addresses.""" + + if self.is_new() and frappe.db.exists("Mail Agent", self.agent): + frappe.throw( + _( + "Mail Agent {0} already exists.".format( + frappe.bold(self.agent), + ) + ) + ) + + ipv4 = get_dns_record(self.agent, "A") + ipv6 = get_dns_record(self.agent, "AAAA") + + self.ipv4 = ipv4[0].address if ipv4 else None + self.ipv6 = ipv6[0].address if ipv6 else None + + +def create_or_update_spf_dns_record(spf_host: str | None = None) -> None: + """Refreshes the SPF DNS Record.""" + + mail_settings = frappe.get_single("Mail Settings") + spf_host = spf_host or mail_settings.spf_host + outbound_agents = frappe.db.get_all( + "Mail Agent", + filters={"enabled": 1, "type": "Outbound"}, + pluck="agent", + order_by="agent asc", + ) + + if not outbound_agents: + if spf_dns_record := frappe.db.exists( + "DNS Record", {"host": spf_host, "type": "TXT"} + ): + frappe.delete_doc("DNS Record", spf_dns_record, ignore_permissions=True) + return + + create_or_update_dns_record( + host=spf_host, + type="TXT", + value=f"v=spf1 {' '.join(outbound_agents)} ~all", + ttl=mail_settings.default_ttl, + category="Server Record", + ) diff --git a/mail/mail/doctype/mail_agent/test_mail_agent.py b/mail/mail/doctype/mail_agent/test_mail_agent.py new file mode 100644 index 00000000..b2aa2a4e --- /dev/null +++ b/mail/mail/doctype/mail_agent/test_mail_agent.py @@ -0,0 +1,9 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +# import frappe +from frappe.tests.utils import FrappeTestCase + + +class TestMailAgent(FrappeTestCase): + pass diff --git a/mail/mail/doctype/mail_domain/mail_domain.py b/mail/mail/doctype/mail_domain/mail_domain.py index bb56d994..12ed9cc6 100644 --- a/mail/mail/doctype/mail_domain/mail_domain.py +++ b/mail/mail/doctype/mail_domain/mail_domain.py @@ -146,8 +146,8 @@ def get_receiving_records(self, ttl: int) -> list[dict]: records = [] if inbound_agents := frappe.db.get_all( "Mail Agent", - filters={"type": "Inbound"}, - fields=["host", "priority"], + filters={"enabled": 1, "type": "Inbound"}, + fields=["agent", "priority"], order_by="priority asc", ): for inbound_agent in inbound_agents: @@ -156,7 +156,7 @@ def get_receiving_records(self, ttl: int) -> list[dict]: "category": "Receiving Record", "type": "MX", "host": self.domain_name, - "value": f"{inbound_agent.host.split(':')[0]}.", + "value": f"{inbound_agent.agent.split(':')[0]}.", "priority": inbound_agent.priority, "ttl": ttl, } diff --git a/mail/mail/doctype/mail_settings/mail_settings.json b/mail/mail/doctype/mail_settings/mail_settings.json index d5ef656b..a17f00f5 100644 --- a/mail/mail/doctype/mail_settings/mail_settings.json +++ b/mail/mail/doctype/mail_settings/mail_settings.json @@ -18,8 +18,6 @@ "default_ttl", "column_break_xlun", "default_newsletter_retention", - "mail_agents_section", - "mail_agents", "rabbitmq_amqp_tab", "rmq_host", "rmq_port", @@ -277,16 +275,6 @@ "label": "Required Spam Score", "reqd": 1 }, - { - "fieldname": "mail_agents", - "fieldtype": "Table", - "options": "Mail Agent" - }, - { - "fieldname": "mail_agents_section", - "fieldtype": "Section Break", - "label": "Mail Agents" - }, { "fieldname": "section_break_71vy", "fieldtype": "Section Break" @@ -312,7 +300,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2024-10-08 22:42:18.860286", + "modified": "2024-10-10 10:51:33.423792", "modified_by": "Administrator", "module": "Mail", "name": "Mail Settings", diff --git a/mail/mail/doctype/mail_settings/mail_settings.py b/mail/mail/doctype/mail_settings/mail_settings.py index 5c1bd4b2..33acdc79 100644 --- a/mail/mail/doctype/mail_settings/mail_settings.py +++ b/mail/mail/doctype/mail_settings/mail_settings.py @@ -8,7 +8,6 @@ from frappe.model.document import Document from mail.utils.validation import is_valid_host from frappe.core.api.file import get_max_file_size -from mail.mail.doctype.dns_record.dns_record import create_or_update_dns_record class MailSettings(Document): @@ -18,8 +17,6 @@ def validate(self) -> None: self.validate_spf_host() self.validate_postmaster() self.validate_default_dkim_key_size() - self.validate_mail_agents() - self.refresh_spf_dns_record() self.validate_rmq_host() self.validate_outgoing_max_attachment_size() self.validate_outgoing_total_attachments_size() @@ -72,6 +69,10 @@ def validate_spf_host(self) -> None: ): frappe.delete_doc("DNS Record", spf_dns_record, ignore_permissions=True) + from mail.mail.doctype.mail_agent.mail_agent import create_or_update_spf_dns_record + + create_or_update_spf_dns_record(self.spf_host) + def validate_postmaster(self) -> None: """Validates the Postmaster.""" @@ -97,41 +98,6 @@ def validate_default_dkim_key_size(self) -> None: if cint(self.default_dkim_key_size) < 1024: frappe.throw(_("DKIM Key Size must be greater than 1024.")) - def validate_mail_agents(self) -> None: - """Validates the Mail Agents.""" - - inbound_agents = [] - outbound_agents = [] - for agent in self.mail_agents: - if agent.type == "Inbound": - inbound_agents.append(agent) - elif agent.type == "Outbound": - outbound_agents.append(agent) - - if not inbound_agents: - frappe.throw(_("At least one Inbound Mail Agent is required.")) - if not outbound_agents: - frappe.throw(_("At least one Outbound Mail Agent is required.")) - - def refresh_spf_dns_record(self, save: bool = False) -> None: - """Refreshes the SPF DNS Record.""" - - if outbound_agents := [ - f"a:{outbound_agent.host}" - for outbound_agent in self.mail_agents - if outbound_agent.type == "Outbound" - ]: - create_or_update_dns_record( - host=self.spf_host, - type="TXT", - value=f"v=spf1 {' '.join(outbound_agents)} ~all", - ttl=self.default_ttl, - category="Server Record", - ) - - if save: - self.save() - def validate_rmq_host(self) -> None: """Validates the rmq_host and converts it to lowercase.""" From 548afd46a028891c6661f1dee00562010a88abae Mon Sep 17 00:00:00 2001 From: s-aga-r Date: Thu, 10 Oct 2024 12:47:56 +0530 Subject: [PATCH 2/2] chore: `linter` --- mail/mail/doctype/mail_agent/mail_agent.py | 8 +------- mail/mail/doctype/mail_settings/mail_settings.py | 8 +++----- mail/utils/__init__.py | 4 ++-- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/mail/mail/doctype/mail_agent/mail_agent.py b/mail/mail/doctype/mail_agent/mail_agent.py index 82348b91..10da7145 100644 --- a/mail/mail/doctype/mail_agent/mail_agent.py +++ b/mail/mail/doctype/mail_agent/mail_agent.py @@ -35,13 +35,7 @@ def validate_agent(self) -> None: """Validates the agent and fetches the IP addresses.""" if self.is_new() and frappe.db.exists("Mail Agent", self.agent): - frappe.throw( - _( - "Mail Agent {0} already exists.".format( - frappe.bold(self.agent), - ) - ) - ) + frappe.throw(_("Mail Agent {0} already exists.").format(frappe.bold(self.agent))) ipv4 = get_dns_record(self.agent, "A") ipv6 = get_dns_record(self.agent, "AAAA") diff --git a/mail/mail/doctype/mail_settings/mail_settings.py b/mail/mail/doctype/mail_settings/mail_settings.py index 33acdc79..868a91e7 100644 --- a/mail/mail/doctype/mail_settings/mail_settings.py +++ b/mail/mail/doctype/mail_settings/mail_settings.py @@ -56,10 +56,8 @@ def validate_spf_host(self) -> None: self.spf_host = self.spf_host.lower() if not is_valid_host(self.spf_host): msg = _( - "SPF Host {0} is invalid. It can be alphanumeric but should not contain spaces or special characters, excluding underscores.".format( - frappe.bold(self.spf_host) - ) - ) + "SPF Host {0} is invalid. It can be alphanumeric but should not contain spaces or special characters, excluding underscores." + ).format(frappe.bold(self.spf_host)) frappe.throw(msg) previous_doc = self.get_doc_before_save() @@ -197,5 +195,5 @@ def validate_mail_settings() -> None: if not mail_settings.get(field): field_label = frappe.get_meta("Mail Settings").get_label(field) frappe.throw( - _("Please set the {0} in the Mail Settings.".format(frappe.bold(field_label))) + _("Please set the {0} in the Mail Settings.").format(frappe.bold(field_label)) ) diff --git a/mail/utils/__init__.py b/mail/utils/__init__.py index f211b26d..194908c0 100644 --- a/mail/utils/__init__.py +++ b/mail/utils/__init__.py @@ -24,9 +24,9 @@ def get_dns_record( return resolver.resolve(fqdn, type) except dns.resolver.NXDOMAIN: - err_msg = _("{0} does not exist.".format(frappe.bold(fqdn))) + err_msg = _("{0} does not exist.").format(frappe.bold(fqdn)) except dns.resolver.NoAnswer: - err_msg = _("No answer for {0}.".format(frappe.bold(fqdn))) + err_msg = _("No answer for {0}.").format(frappe.bold(fqdn)) except dns.exception.DNSException as e: err_msg = _(str(e))