vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

chore(communication): cleanup unused code (#28199)

Browse source 
* chore: feedback doctype no longer exists

missed from https://github.com/frappe/frappe/pull/17479

* chore: remove unused communication type

This was removed and migrated already in v12:

```
frappe/patches/v12_0/setup_comments_from_communications.py:	frappe.db.delete("Communication", {"communication_type": "Comment"})
```
... comming from 41d90fa6d1
which effectively reverted 465318878e
This commit is contained in:
David Arnold 2024-11-14 00:12:14 +01:00 • committed by GitHub
parent 786106d45e
commit 057139ea2e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 124 additions and 158 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
frappe/core/doctype/communication/communication.json
View file

@ -24,7 +24,6 @@
"status_section",
"text_content",
"communication_type",
"comment_type",
"column_break_5",
"status",
"sent_or_received",
@ -55,10 +54,7 @@
"uid",
"imap_folder",
"email_status",
"has_attachment",
"feedback_section",
"rating",
"feedback_request"
"has_attachment"
],
"fields": [
{
@ -75,9 +71,10 @@
"label": "To and CC"
},
{
"depends_on": "eval:doc.communication_type===\"Communication\"",
"fieldname": "communication_medium",
"fieldtype": "Select",
"in_list_view": 1,
"in_standard_filter": 1,
"label": "Type",
"options": "\nEmail\nChat\nPhone\nSMS\nEvent\nMeeting\nVisit\nOther"
},
@ -154,20 +151,12 @@
"default": "Communication",
"fieldname": "communication_type",
"fieldtype": "Select",
"label": "Communication Type",
"options": "Communication\nComment\nChat\nNotification\nFeedback\nAutomated Message",
"read_only": 1,
"reqd": 1
},
{
"fieldname": "comment_type",
"fieldtype": "Select",
"hidden": 1,
"in_list_view": 1,
"in_standard_filter": 1,
"label": "Comment Type",
"options": "\nComment\nLike\nInfo\nLabel\nWorkflow\nCreated\nSubmitted\nCancelled\nUpdated\nDeleted\nAssigned\nAssignment Completed\nAttachment\nAttachment Removed\nShared\nUnshared\nRelinked",
"read_only": 1
"label": "Communication Type",
"options": "Communication\nAutomated Message",
"read_only": 1,
"reqd": 1
},
{
"fieldname": "column_break_5",
@ -346,25 +335,6 @@
"hidden": 1,
"label": "Has Attachment"
},
{
"collapsible": 1,
"depends_on": "eval: doc.rating > 0",
"fieldname": "feedback_section",
"fieldtype": "Section Break",
"label": "Feedback"
},
{
"fieldname": "rating",
"fieldtype": "Int",
"label": "Rating",
"read_only": 1
},
{
"fieldname": "feedback_request",
"fieldtype": "Data",
"label": "Feedback Request",
"read_only": 1
},
{
"fieldname": "email_template",
"fieldtype": "Link",
@ -466,4 +436,4 @@
"title_field": "subject",
"track_changes": 1,
"track_seen": 1
}
}

56
frappe/core/doctype/communication/communication.py
View file

@ -44,33 +44,11 @@ class Communication(Document, CommunicationEmailMixin):
_user_tags: DF.Data | None
bcc: DF.Code | None
cc: DF.Code | None
comment_type: DF.Literal[
"",
"Comment",
"Like",
"Info",
"Label",
"Workflow",
"Created",
"Submitted",
"Cancelled",
"Updated",
"Deleted",
"Assigned",
"Assignment Completed",
"Attachment",
"Attachment Removed",
"Shared",
"Unshared",
"Relinked",
]
communication_date: DF.Datetime | None
communication_medium: DF.Literal[
"", "Email", "Chat", "Phone", "SMS", "Event", "Meeting", "Visit", "Other"
]
communication_type: DF.Literal[
"Communication", "Comment", "Chat", "Notification", "Feedback", "Automated Message"
]
communication_type: DF.Literal["Communication", "Automated Message"]
content: DF.TextEditor | None
delivery_status: DF.Literal[
"",
@ -92,13 +70,11 @@ class Communication(Document, CommunicationEmailMixin):
email_account: DF.Link | None
email_status: DF.Literal["Open", "Spam", "Trash"]
email_template: DF.Link | None
feedback_request: DF.Data | None
has_attachment: DF.Check
imap_folder: DF.Data | None
in_reply_to: DF.Link | None
message_id: DF.SmallText | None
phone_no: DF.Data | None
rating: DF.Int
read_by_recipient: DF.Check
read_by_recipient_on: DF.Datetime | None
read_receipt: DF.Check
@ -218,19 +194,7 @@ class Communication(Document, CommunicationEmailMixin):
if self.reference_doctype == "Communication" and self.sent_or_received == "Sent":
frappe.db.set_value("Communication", self.reference_name, "status", "Replied")
if self.communication_type == "Communication":
self.notify_change("add")
elif self.communication_type in ("Chat", "Notification"):
if self.reference_name == frappe.session.user:
message = self.as_dict()
message["broadcast"] = True
frappe.publish_realtime("new_message", message, after_commit=True)
else:
# reference_name contains the user who is addressed in the messages' page comment
frappe.publish_realtime(
"new_message", self.as_dict(), user=self.reference_name, after_commit=True
)
self.notify_change("add")
def set_signature_in_email_content(self):
"""Set sender's User.email_signature or default outgoing's EmailAccount.signature to the email"""
@ -284,13 +248,10 @@ class Communication(Document, CommunicationEmailMixin):
if (method := getattr(parent, "on_communication_update", None)) and callable(method):
parent.on_communication_update(self)
return
if self.comment_type != "Updated":
update_parent_document_on_communication(self)
update_parent_document_on_communication(self)
def on_trash(self):
if self.communication_type == "Communication":
self.notify_change("delete")
self.notify_change("delete")
@property
def sender_mailid(self):
@ -342,10 +303,8 @@ class Communication(Document, CommunicationEmailMixin):
def set_status(self):
if self.reference_doctype and self.reference_name:
self.status = "Linked"
elif self.communication_type == "Communication":
self.status = "Open"
else:
self.status = "Closed"
self.status = "Open"
if self.send_after and self.is_new():
self.delivery_status = "Scheduled"
@ -658,11 +617,6 @@ def update_parent_document_on_communication(doc):
if not parent:
return
# update parent mins_to_first_communication only if we create the Email communication
# ignore in case of only Comment is added
if doc.communication_type == "Comment":
return
status_field = parent.meta.get_field("status")
if status_field:
options = (status_field.options or "").splitlines()

11
frappe/core/doctype/user/user.py
View file

@ -547,17 +547,6 @@ class User(Document):
# delete shares
frappe.db.delete("DocShare", {"user": self.name})
# delete messages
table = DocType("Communication")
frappe.db.delete(
table,
filters=(
(table.communication_type.isin(["Chat", "Notification"]))
& (table.reference_doctype == "User")
& ((table.reference_name == self.name) | table.owner == self.name)
),
run=False,
)
# unlink contact
table = DocType("Contact")
frappe.qb.update(table).where(table.user == self.name).set(table.user, None).run()

83
frappe/desk/doctype/dashboard_chart/test_dashboard_chart.py
View file

@ -7,6 +7,7 @@ from unittest.mock import patch
from dateutil.relativedelta import relativedelta
import frappe
from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.desk.doctype.dashboard_chart.dashboard_chart import get
from frappe.tests import IntegrationTestCase, UnitTestCase
from frappe.utils import formatdate, get_last_day, getdate
@ -23,6 +24,32 @@ class UnitTestDashboardChart(UnitTestCase):
class TestDashboardChart(IntegrationTestCase):
def setUp(self):
doc = new_doctype(
fields=[
{
"fieldname": "title",
"fieldtype": "Text",
"label": "Title",
"reqd": 1, # mandatory
},
{
"fieldname": "number",
"fieldtype": "Int",
"label": "Number",
"reqd": 1, # mandatory
},
{
"fieldname": "date",
"fieldtype": "Date",
"label": "Date",
"reqd": 1, # mandatory
},
],
)
doc.insert()
self.doctype_name = doc.name
def test_period_ending(self):
self.assertEqual(get_period_ending("2019-04-10", "Daily"), getdate("2019-04-10"))
@ -147,7 +174,7 @@ class TestDashboardChart(IntegrationTestCase):
self.assertEqual(result.get("datasets")[0].get("values")[0], todo_status_count)
def test_daily_dashboard_chart(self):
insert_test_records()
insert_test_records(self.doctype_name)
if frappe.db.exists("Dashboard Chart", "Test Daily Dashboard Chart"):
frappe.delete_doc("Dashboard Chart", "Test Daily Dashboard Chart")
@ -156,9 +183,9 @@ class TestDashboardChart(IntegrationTestCase):
doctype="Dashboard Chart",
chart_name="Test Daily Dashboard Chart",
chart_type="Sum",
document_type="Communication",
based_on="communication_date",
value_based_on="rating",
document_type=self.doctype_name,
based_on="date",
value_based_on="number",
timespan="Select Date Range",
time_interval="Daily",
from_date=datetime(2019, 1, 6),
@ -176,7 +203,7 @@ class TestDashboardChart(IntegrationTestCase):
)
def test_weekly_dashboard_chart(self):
insert_test_records()
insert_test_records(self.doctype_name)
if frappe.db.exists("Dashboard Chart", "Test Weekly Dashboard Chart"):
frappe.delete_doc("Dashboard Chart", "Test Weekly Dashboard Chart")
@ -185,9 +212,9 @@ class TestDashboardChart(IntegrationTestCase):
doctype="Dashboard Chart",
chart_name="Test Weekly Dashboard Chart",
chart_type="Sum",
document_type="Communication",
based_on="communication_date",
value_based_on="rating",
document_type=self.doctype_name,
based_on="date",
value_based_on="number",
timespan="Select Date Range",
time_interval="Weekly",
from_date=datetime(2018, 12, 30),
@ -203,7 +230,7 @@ class TestDashboardChart(IntegrationTestCase):
self.assertEqual(result.get("labels"), ["12-30-2018", "01-06-2019", "01-13-2019", "01-20-2019"])
def test_avg_dashboard_chart(self):
insert_test_records()
insert_test_records(self.doctype_name)
if frappe.db.exists("Dashboard Chart", "Test Average Dashboard Chart"):
frappe.delete_doc("Dashboard Chart", "Test Average Dashboard Chart")
@ -212,9 +239,9 @@ class TestDashboardChart(IntegrationTestCase):
doctype="Dashboard Chart",
chart_name="Test Average Dashboard Chart",
chart_type="Average",
document_type="Communication",
based_on="communication_date",
value_based_on="rating",
document_type=self.doctype_name,
based_on="date",
value_based_on="number",
timespan="Select Date Range",
time_interval="Weekly",
from_date=datetime(2018, 12, 30),
@ -254,22 +281,22 @@ class TestDashboardChart(IntegrationTestCase):
self.assertEqual(sorted(result.get("labels")), sorted(["01-19-2019", "01-05-2019", "01-12-2019"]))
def insert_test_records():
create_new_communication("Communication 1", datetime(2018, 12, 30), 50)
create_new_communication("Communication 2", datetime(2019, 1, 4), 100)
create_new_communication("Communication 3", datetime(2019, 1, 6), 200)
create_new_communication("Communication 4", datetime(2019, 1, 7), 400)
create_new_communication("Communication 5", datetime(2019, 1, 8), 300)
create_new_communication("Communication 6", datetime(2019, 1, 10), 100)
def insert_test_records(doctype_name):
create_new_record(doctype_name, "Title 1", datetime(2018, 12, 30), 50)
create_new_record(doctype_name, "Title 2", datetime(2019, 1, 4), 100)
create_new_record(doctype_name, "Title 3", datetime(2019, 1, 6), 200)
create_new_record(doctype_name, "Title 4", datetime(2019, 1, 7), 400)
create_new_record(doctype_name, "Title 5", datetime(2019, 1, 8), 300)
create_new_record(doctype_name, "Title 6", datetime(2019, 1, 10), 100)
def create_new_communication(subject, date, rating):
communication = {
"doctype": "Communication",
"subject": subject,
"rating": rating,
"communication_date": date,
def create_new_record(doctype_name, title, date, number):
doc = {
"doctype": doctype_name,
"title": title,
"date": date,
"number": number,
}
comm = frappe.get_doc(communication)
if not frappe.db.exists("Communication", {"subject": comm.subject}):
comm.insert()
doc = frappe.get_doc(doc)
if not frappe.db.exists(doctype_name, {"title": doc.title}):
doc.insert(ignore_mandatory=True)

8
frappe/desk/form/load.py
View file

@ -292,11 +292,11 @@ def get_communication_data(
if not fields:
fields = """
C.name, C.communication_type, C.communication_medium,
C.comment_type, C.communication_date, C.content,
C.communication_date, C.content,
C.sender, C.sender_full_name, C.cc, C.bcc,
C.creation AS creation, C.subject, C.delivery_status,
C._liked_by, C.reference_doctype, C.reference_name,
C.read_by_recipient, C.rating, C.recipients
C.read_by_recipient, C.recipients
"""
conditions = ""
@ -315,7 +315,7 @@ def get_communication_data(
part1 = f"""
SELECT {fields}
FROM `tabCommunication` as C
WHERE C.communication_type IN ('Communication', 'Feedback', 'Automated Message')
WHERE C.communication_type IN ('Communication', 'Automated Message')
AND (C.reference_doctype = %(doctype)s AND C.reference_name = %(name)s)
{conditions}
"""
@ -325,7 +325,7 @@ def get_communication_data(
SELECT {fields}
FROM `tabCommunication` as C
INNER JOIN `tabCommunication Link` ON C.name=`tabCommunication Link`.parent
WHERE C.communication_type IN ('Communication', 'Feedback', 'Automated Message')
WHERE C.communication_type IN ('Communication', 'Automated Message')
AND `tabCommunication Link`.link_doctype = %(doctype)s AND `tabCommunication Link`.link_name = %(name)s
{conditions}
"""

27
frappe/email/doctype/email_account/test_email_account.py
View file

@ -168,16 +168,23 @@ class TestEmailAccount(IntegrationTestCase):
)
def test_outgoing(self):
make(
comm_name = make(
subject="test-mail-000",
content="test mail 000",
recipients="test_receiver@example.com",
send_email=True,
sender="test_sender@example.com",
)
)["name"]
mail = email.message_from_string(frappe.get_last_doc("Email Queue").message)
self.assertTrue("test-mail-000" in mail.get("Subject"))
sent_mail = email.message_from_string(
frappe.get_doc(
"Email Queue",
{
"communication": comm_name,
},
).message
)
self.assertTrue("test-mail-000" in sent_mail.get("Subject"))
def test_sendmail(self):
frappe.sendmail(
@ -192,7 +199,7 @@ class TestEmailAccount(IntegrationTestCase):
self.assertTrue("test-mail-001" in sent_mail.get("Subject"))
def test_print_format(self):
make(
comm_name = make(
sender="test_sender@example.com",
recipients="test_recipient@example.com",
content="test mail 001",
@ -201,9 +208,15 @@ class TestEmailAccount(IntegrationTestCase):
name="_Test Email Account 1",
print_format="Standard",
send_email=True,
)["name"]
sent_mail = email.message_from_string(
frappe.get_doc(
"Email Queue",
{
"communication": comm_name,
},
).message
)
sent_mail = email.message_from_string(frappe.get_last_doc("Email Queue").message)
self.assertTrue("test-mail-002" in sent_mail.get("Subject"))
def test_threading(self):

3
frappe/email/doctype/notification/test_notification.py
View file

@ -68,7 +68,8 @@ class TestNotification(IntegrationTestCase):
def test_new_and_save(self):
"""Check creating a new communication triggers a notification."""
communication = frappe.new_doc("Communication")
communication.communication_type = "Comment"
communication.communication_type = "Communication"
communication.sender_full_name = "__test_notification_sender__"
communication.subject = "test"
communication.content = "test"
communication.insert(ignore_permissions=True)

9
frappe/email/doctype/notification/test_records.toml
View file

@ -3,8 +3,8 @@ subject = "_Test Notification 1"
document_type = "Communication"
event = "New"
attach_print = 0
message = "New comment {{ doc.content }} created"
condition = "doc.communication_type=='Comment'"
message = "New communication {{ doc.content }} created"
condition = "doc.communication_type=='Communication' and doc.sender_full_name=='__test_notification_sender__'"
[[Notification.recipients]]
receiver_by_document_field = "owner"
@ -14,8 +14,9 @@ subject = "_Test Notification 2"
document_type = "Communication"
event = "Save"
attach_print = 0
message = "New comment {{ doc.content }} saved"
condition = "doc.communication_type=='Comment'"
sender_full_name = "__testsender__"
message = "New communication {{ doc.content }} saved"
condition = "doc.communication_type=='Communication' and doc.sender_full_name=='__test_notification_sender__'"
set_property_after_alert = "subject"
property_value = "__testing__"
[[Notification.recipients]]

39
frappe/tests/test_query_builder.py
View file

@ -3,6 +3,7 @@ from collections.abc import Callable
from datetime import time
import frappe
from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.query_builder import Case
from frappe.query_builder.builder import Function
from frappe.query_builder.custom import ConstantColumn
@ -316,22 +317,32 @@ class TestBuilderBase:
self.assertIsInstance(data, list)
def test_agg_funcs(self):
frappe.db.truncate("Communication")
doc = new_doctype(
fields=[
{
"fieldname": "number",
"fieldtype": "Int",
"label": "Number",
"reqd": 1, # mandatory
},
],
)
doc.insert()
self.doctype_name = doc.name
frappe.db.truncate(self.doctype_name)
sample_data = {
"doctype": "Communication",
"communication_type": "Communication",
"content": "testing",
"rating": 1,
"doctype": self.doctype_name,
"number": 1,
}
frappe.get_doc(sample_data).insert()
sample_data["rating"] = 3
frappe.get_doc(sample_data).insert()
sample_data["rating"] = 4
frappe.get_doc(sample_data).insert()
self.assertEqual(frappe.qb.max("Communication", "rating"), 4)
self.assertEqual(frappe.qb.min("Communication", "rating"), 1)
self.assertAlmostEqual(frappe.qb.avg("Communication", "rating"), 2.666, places=2)
self.assertEqual(frappe.qb.sum("Communication", "rating"), 8.0)
frappe.get_doc(sample_data).insert(ignore_mandatory=True)
sample_data["number"] = 3
frappe.get_doc(sample_data).insert(ignore_mandatory=True)
sample_data["number"] = 4
frappe.get_doc(sample_data).insert(ignore_mandatory=True)
self.assertEqual(frappe.qb.max(self.doctype_name, "number"), 4)
self.assertEqual(frappe.qb.min(self.doctype_name, "number"), 1)
self.assertAlmostEqual(frappe.qb.avg(self.doctype_name, "number"), 2.666, places=2)
self.assertEqual(frappe.qb.sum(self.doctype_name, "number"), 8.0)
frappe.db.rollback()