diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 0fde168532..5632f05a36 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -10,8 +10,13 @@ from frappe.query_builder.functions import Now class LogSettings(Document): - def clear_logs(self): + def clear_logs(self, commit=False): self.clear_email_queue() + if commit: + # Since since deleting many logs can take significant amount of time, commit is required to relase locks. + # Error log table doesn't require commit - myisam + # activity logs are deleted last so background job finishes and commits. + frappe.db.commit() self.clear_error_logs() self.clear_activity_logs() @@ -34,7 +39,7 @@ class LogSettings(Document): def run_log_clean_up(): doc = frappe.get_doc("Log Settings") - doc.clear_logs() + doc.clear_logs(commit=True) @frappe.whitelist() diff --git a/frappe/email/doctype/email_queue/test_email_queue.py b/frappe/email/doctype/email_queue/test_email_queue.py index b3c5467085..96c566a041 100644 --- a/frappe/email/doctype/email_queue/test_email_queue.py +++ b/frappe/email/doctype/email_queue/test_email_queue.py @@ -1,10 +1,41 @@ # -*- coding: utf-8 -*- # Copyright (c) 2015, Frappe Technologies and Contributors # License: MIT. See LICENSE -import unittest -# test_records = frappe.get_test_records('Email Queue') +import frappe +from frappe.email.queue import clear_outbox +from frappe.tests.utils import FrappeTestCase -class TestEmailQueue(unittest.TestCase): - pass +class TestEmailQueue(FrappeTestCase): + def test_email_queue_deletion_based_on_modified_date(self): + old_record = frappe.get_doc( + { + "doctype": "Email Queue", + "sender": "Test ", + "show_as_cc": "", + "message": "Test message", + "status": "Sent", + "priority": 1, + "recipients": [ + { + "recipient": "test_auth@test.com", + } + ], + } + ).insert() + + old_record.modified = "2010-01-01 00:00:01" + old_record.recipients[0].modified = old_record.modified + old_record.db_update_all() + + new_record = frappe.copy_doc(old_record) + new_record.insert() + + clear_outbox() + + self.assertFalse(frappe.db.exists("Email Queue", old_record.name)) + self.assertFalse(frappe.db.exists("Email Queue Recipient", {"parent": old_record.name})) + + self.assertTrue(frappe.db.exists("Email Queue", new_record.name)) + self.assertTrue(frappe.db.exists("Email Queue Recipient", {"parent": new_record.name})) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index b92dea3e65..07731417d8 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -195,18 +195,24 @@ def clear_outbox(days: int = None) -> None: Note: Used separate query to avoid deadlock """ days = days or 31 - email_queue = DocType("Email Queue") + email_queue = frappe.qb.DocType("Email Queue") + email_recipient = frappe.qb.DocType("Email Queue Recipient") - email_queues = ( + # Delete queue table + ( frappe.qb.from_(email_queue) - .select(email_queue.name) - .where(email_queue.modified < (Now() - Interval(days=days))) - .run(pluck=True) - ) + .delete() + .where((email_queue.modified < (Now() - Interval(days=days)))) + ).run() - if email_queues: - frappe.db.delete("Email Queue", {"name": ("in", email_queues)}) - frappe.db.delete("Email Queue Recipient", {"parent": ("in", email_queues)}) + # delete child tables, note that this has potential to leave some orphan + # child table behind if modified time was later than parent doc (rare). + # But it's safe since child table doesn't contain links. + ( + frappe.qb.from_(email_recipient) + .delete() + .where((email_recipient.modified < (Now() - Interval(days=days)))) + ).run() def set_expiry_for_email_queue():