diff --git a/frappe/contacts/doctype/contact/contact.json b/frappe/contacts/doctype/contact/contact.json index 85986dd9d5..7bc13accf8 100644 --- a/frappe/contacts/doctype/contact/contact.json +++ b/frappe/contacts/doctype/contact/contact.json @@ -21,6 +21,7 @@ "designation", "gender", "phone", + "mobile_no", "company_name", "image", "sb_00", @@ -192,9 +193,15 @@ { "fieldname": "phone_nos", "fieldtype": "Table", - "label": "Phone Nos", + "label": "Numbers", "options": "Contact Phone" }, + { + "fieldname": "mobile_no", + "fieldtype": "Data", + "label": "Mobile No", + "read_only": 1 + }, { "default": "0", "fieldname": "pulled_from_google_contacts", @@ -238,8 +245,8 @@ "icon": "fa fa-user", "idx": 1, "image_field": "image", - "modified": "2019-09-13 15:50:38.999884", - "modified_by": "himanshu@erpnext.com", + "modified": "2019-09-24 17:48:26.790985", + "modified_by": "Administrator", "module": "Contacts", "name": "Contact", "name_case": "Title Case", diff --git a/frappe/contacts/doctype/contact/contact.py b/frappe/contacts/doctype/contact/contact.py index 0b1231e511..f851e19b46 100644 --- a/frappe/contacts/doctype/contact/contact.py +++ b/frappe/contacts/doctype/contact/contact.py @@ -29,11 +29,10 @@ class Contact(Document): break def validate(self): - self.set_primary("email_id", "email_ids") - self.set_primary("phone", "phone_nos") - - if self.email_id: - self.email_id = self.email_id.strip() + self.set_primary_email() + self.set_primary("phone") + self.set_primary("mobile_no") + self.check_if_primary_phone_and_mobile_no_same() self.set_user() @@ -79,24 +78,51 @@ class Contact(Document): if autosave: self.save(ignore_permissions=True) - def add_phone(self, phone, is_primary=0, autosave=False): + def add_phone(self, phone, is_primary_phone=0, is_primary_mobile_no=0, autosave=False): self.append("phone_nos", { "phone": phone, - "is_primary": is_primary + "is_primary_phone": is_primary_phone, + "is_primary_mobile_no": is_primary_mobile_no }) if autosave: self.save(ignore_permissions=True) - def set_primary(self, fieldname, child_table): - if len(self.get(child_table)) == 1: - self.get(child_table)[0].is_primary = 1 - setattr(self, fieldname, self.get(child_table)[0].get(fieldname)) - else: - for d in self.get(child_table): - if d.is_primary == 1: - setattr(self, fieldname, d.get(fieldname)) - break + def set_primary_email(self): + if not self.email_ids: + self.email_id = "" + return + + if len([email.email_id for email in self.email_ids if email.is_primary]) > 1: + frappe.throw(_("Only one {0} can be set as primary.").format(frappe.bold("Email ID"))) + + for d in self.email_ids: + if d.is_primary == 1: + self.email_id = d.email_id.strip() + break + + def set_primary(self, fieldname): + # Used to set primary mobile and phone no. + if len(self.phone_nos) == 0: + setattr(self, fieldname, "") + return + + field_name = "is_primary_" + fieldname + + is_primary = [phone.phone for phone in self.phone_nos if phone.get(field_name)] + + if len(is_primary) > 1: + frappe.throw(_("Only one {0} can be set as primary.").format(frappe.bold(frappe.unscrub(fieldname)))) + + for d in self.phone_nos: + if d.get(field_name) == 1: + setattr(self, fieldname, d.phone) + break + + def check_if_primary_phone_and_mobile_no_same(self): + if self.phone and self.mobile_no and self.phone == self.mobile_no: + number = frappe.bold(self.phone) + frappe.throw(_("Number {0} cannot be set as primary for Phone as well as Mobile No.").format(number)) def get_default_contact(doctype, name): '''Returns default contact for the given doctype, name''' diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index 496ff68299..2e6f9ad422 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -5,8 +5,82 @@ from __future__ import unicode_literals import frappe import unittest - -test_records = frappe.get_test_records('Contact') +from frappe.exceptions import ValidationError class TestContact(unittest.TestCase): - pass + + def test_check_default_email(self): + emails = [ + {"email": "test1@example.com", "is_primary": 0}, + {"email": "test2@example.com", "is_primary": 0}, + {"email": "test3@example.com", "is_primary": 0}, + {"email": "test4@example.com", "is_primary": 1}, + {"email": "test5@example.com", "is_primary": 0}, + ] + contact = create_contact("Email", "Mr", emails=emails) + + self.assertEqual(contact.email_id, "test4@example.com") + + def test_check_default_phone_and_mobile(self): + phones = [ + {"phone": "+91 0000000000", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000001", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000002", "is_primary_phone": 1, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000003", "is_primary_phone": 0, "is_primary_mobile_no": 1}, + ] + contact = create_contact("Phone", "Mr", phones=phones) + + self.assertEqual(contact.phone, "+91 0000000002") + self.assertEqual(contact.mobile_no, "+91 0000000003") + + def test_same_phone_and_mobile(self): + phones = [ + {"phone": "+91 0000000000", "is_primary_phone": 1, "is_primary_mobile_no": 1}, + ] + contact = create_contact("Phone", "Mr", phones=phones, save=False) + self.assertRaises(ValidationError, contact.save) + + def test_no_primary_set(self): + emails = [ + {"email": "test1@example.com", "is_primary": 0}, + {"email": "test2@example.com", "is_primary": 0}, + {"email": "test3@example.com", "is_primary": 0}, + {"email": "test4@example.com", "is_primary": 0}, + {"email": "test5@example.com", "is_primary": 0}, + ] + phones = [ + {"phone": "+91 0000000000", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000001", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + {"phone": "+91 0000000002", "is_primary_phone": 1, "is_primary_mobile_no": 1}, + {"phone": "+91 0000000003", "is_primary_phone": 0, "is_primary_mobile_no": 0}, + ] + + contact_email = create_contact("Default", "Mr", emails=emails, phones=phones, save=False) + contact_phone = create_contact("Default", "Mr", emails=emails, phones=phones, save=False) + + # No default set for emails if many emails are passed as params + self.assertRaises(ValidationError, contact_email.save) + + # No default set for phones if many phones are passed as params + self.assertRaises(ValidationError, contact_phone.save) + +def create_contact(name, salutation, emails=None, phones=None, save=True): + doc = frappe.get_doc({ + "doctype": "Contact", + "first_name": name, + "status": "Open", + "salutation": salutation + }) + + if emails: + for d in emails: + doc.add_email(d.get("email"), d.get("is_primary")) + + if phones: + for d in phones: + doc.add_phone(d.get("phone"), d.get("is_primary_phone"), d.get("is_primary_mobile_no")) + + if save: + doc.insert() + + return doc \ No newline at end of file diff --git a/frappe/contacts/doctype/contact/test_records.json b/frappe/contacts/doctype/contact/test_records.json index 8f4be113b4..11c5329e93 100644 --- a/frappe/contacts/doctype/contact/test_records.json +++ b/frappe/contacts/doctype/contact/test_records.json @@ -1,19 +1,39 @@ [ - { - "doctype": "Contact", - "salutation": "Mr", - "email_id": "test_contact@example.com", - "first_name": "_Test Contact For _Test Customer", - "is_primary_contact": 1, - "phone": "+91 0000000000", - "status": "Open" - }, - { - "doctype": "Contact", - "email_id": "test_contact@example.com", - "first_name": "_Test Contact For _Test Supplier", - "is_primary_contact": 1, - "phone": "+91 0000000000", - "status": "Open" - } + { + "doctype": "Contact", + "salutation": "Mr", + "first_name": "_Test Contact For _Test Customer", + "is_primary_contact": 1, + "status": "Open", + "email_ids": [ + { + "email_id": "test_contact@example.com", + "is_primary": 1 + } + ], + "phone_nos": [ + { + "phone": "+91 0000000000", + "is_primary_phone": 1 + } + ] + }, + { + "doctype": "Contact", + "first_name": "_Test Contact For _Test Supplier", + "is_primary_contact": 1, + "status": "Open", + "email_ids": [ + { + "email_id": "test_contact@example.com", + "is_primary": 1 + } + ], + "phone_nos": [ + { + "phone": "+91 0000000000", + "is_primary_phone": 1 + } + ] + } ] \ No newline at end of file diff --git a/frappe/contacts/doctype/contact_email/contact_email.json b/frappe/contacts/doctype/contact_email/contact_email.json index cfcc39ca8b..f36e155a75 100644 --- a/frappe/contacts/doctype/contact_email/contact_email.json +++ b/frappe/contacts/doctype/contact_email/contact_email.json @@ -13,9 +13,11 @@ "fieldtype": "Data", "in_list_view": 1, "label": "Email ID", - "options": "Email" + "options": "Email", + "reqd": 1 }, { + "columns": 2, "default": "0", "fieldname": "is_primary", "fieldtype": "Check", @@ -24,7 +26,7 @@ } ], "istable": 1, - "modified": "2019-08-02 13:14:22.193463", + "modified": "2019-09-24 17:47:30.565805", "modified_by": "Administrator", "module": "Contacts", "name": "Contact Email", diff --git a/frappe/contacts/doctype/contact_phone/contact_phone.json b/frappe/contacts/doctype/contact_phone/contact_phone.json index 971dedf3d2..3fb203ed69 100644 --- a/frappe/contacts/doctype/contact_phone/contact_phone.json +++ b/frappe/contacts/doctype/contact_phone/contact_phone.json @@ -5,25 +5,36 @@ "engine": "InnoDB", "field_order": [ "phone", - "is_primary" + "is_primary_phone", + "is_primary_mobile_no" ], "fields": [ - { - "default": "0", - "fieldname": "is_primary", - "fieldtype": "Check", - "in_list_view": 1, - "label": "Is Primary" - }, { "fieldname": "phone", "fieldtype": "Data", "in_list_view": 1, - "label": "Phone" + "label": "Number", + "reqd": 1 + }, + { + "columns": 2, + "default": "0", + "fieldname": "is_primary_phone", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Is Primary Phone" + }, + { + "columns": 2, + "default": "0", + "fieldname": "is_primary_mobile_no", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Is Primary Mobile" } ], "istable": 1, - "modified": "2019-08-05 11:40:59.104224", + "modified": "2019-09-24 17:47:50.375326", "modified_by": "Administrator", "module": "Contacts", "name": "Contact Phone", diff --git a/frappe/contacts/report/addresses_and_contacts/addresses_and_contacts.py b/frappe/contacts/report/addresses_and_contacts/addresses_and_contacts.py index fb345a4384..ba0ecd41d1 100644 --- a/frappe/contacts/report/addresses_and_contacts/addresses_and_contacts.py +++ b/frappe/contacts/report/addresses_and_contacts/addresses_and_contacts.py @@ -7,7 +7,7 @@ import frappe from frappe import _ field_map = { - "Contact": ["first_name", "last_name", "address", "phone", "email_id", "is_primary_contact"], + "Contact": ["first_name", "last_name", "address", "phone", "mobile_no", "email_id", "is_primary_contact"], "Address": ["address_line1", "address_line2", "city", "state", "pincode", "country", "is_primary_address"] } diff --git a/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py b/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py index d87676e272..2db395102a 100644 --- a/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py +++ b/frappe/contacts/report/addresses_and_contacts/test_addresses_and_contacts.py @@ -82,8 +82,8 @@ def create_linked_contact(link_list, address): "address": address, "status": "Open" }) - contact.add_email("test_contact@example.com") - contact.add_phone("+91 0000000000") + contact.add_email("test_contact@example.com", is_primary=True) + contact.add_phone("+91 0000000000", is_primary_phone=True) for name in link_list: contact.append("links",{ @@ -103,7 +103,7 @@ class TestAddressesAndContacts(unittest.TestCase): create_linked_contact(links_list, d) report_data = get_data({"reference_doctype": "Test Custom Doctype"}) for idx, link in enumerate(links_list): - test_item = [link, 'test address line 1', 'test address line 2', 'Milan', None, None, 'Italy', 0, '_Test First Name', '_Test Last Name', '_Test Address-Billing', '+91 0000000000', 'test_contact@example.com', 1] + test_item = [link, 'test address line 1', 'test address line 2', 'Milan', None, None, 'Italy', 0, '_Test First Name', '_Test Last Name', '_Test Address-Billing', '+91 0000000000', None, 'test_contact@example.com', 1] self.assertListEqual(test_item, report_data[idx]) def tearDown(self): diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index dcbf3ee35a..6f15e69d30 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1027,9 +1027,10 @@ def update_roles(role_profile): user.add_roles(*roles) def create_contact(user, ignore_links=False, ignore_mandatory=False): + from frappe.contacts.doctype.contact.contact import get_contact_name if user.name in ["Administrator", "Guest"]: return - if not frappe.db.get_value("Contact", {"email_id": user.email}): + if not get_contact_name(user.email): contact = frappe.get_doc({ "doctype": "Contact", "first_name": user.first_name, @@ -1039,7 +1040,7 @@ def create_contact(user, ignore_links=False, ignore_mandatory=False): }) if user.email: - contact.add_email(user.email) + contact.add_email(user.email, is_primary=True) if user.phone: contact.add_phone(user.phone) @@ -1048,7 +1049,6 @@ def create_contact(user, ignore_links=False, ignore_mandatory=False): contact.add_phone(user.mobile_no) contact.insert(ignore_permissions=True, ignore_links=ignore_links, ignore_mandatory=ignore_mandatory) - @frappe.whitelist() def generate_keys(user): """ diff --git a/frappe/hooks.py b/frappe/hooks.py index d2e8570644..a54bff1bda 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -270,7 +270,7 @@ user_privacy_documents = [ { 'doctype': 'Contact', 'match_field': 'email_id', - 'personal_fields': ['first_name', 'last_name', 'phone'], + 'personal_fields': ['first_name', 'last_name', 'phone', 'mobile_no'], }, { 'doctype': 'Contact Email', diff --git a/frappe/integrations/doctype/google_contacts/google_contacts.py b/frappe/integrations/doctype/google_contacts/google_contacts.py index 738c097f63..8c6ab97017 100644 --- a/frappe/integrations/doctype/google_contacts/google_contacts.py +++ b/frappe/integrations/doctype/google_contacts/google_contacts.py @@ -187,7 +187,7 @@ def sync_contacts_from_google_contacts(g_contact): contact.add_email(email_id=email.get("value"), is_primary=1 if email.get("metadata").get("primary") else 0) for phone in connection.get("phoneNumbers", []): - contact.add_phone(phone=phone.get("value"), is_primary=1 if phone.get("metadata").get("primary") else 0) + contact.add_phone(phone=phone.get("value"), is_primary_phone=1 if phone.get("metadata").get("primary") else 0) contact.insert(ignore_permissions=True) diff --git a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py index 64d4e45660..9a62b845e1 100644 --- a/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py +++ b/frappe/website/doctype/personal_data_download_request/test_personal_data_download_request.py @@ -7,15 +7,19 @@ import frappe import unittest import json from frappe.website.doctype.personal_data_download_request.personal_data_download_request import get_user_data - +from frappe.contacts.doctype.contact.contact import get_contact_name class TestRequestPersonalData(unittest.TestCase): def setUp(self): create_user_if_not_exists(email='test_privacy@example.com') + def tearDown(self): + frappe.db.sql("""DELETE FROM `tabPersonal Data Download Request`""") + def test_user_data_creation(self): user_data = json.loads(get_user_data('test_privacy@example.com')) - expected_data = {'Contact': frappe.get_all('Contact', {'email_id':'test_privacy@example.com'}, ["*"])} + contact_name = get_contact_name('test_privacy@example.com') + expected_data = {'Contact': frappe.get_all('Contact', {"name": contact_name}, ["*"])} expected_data = json.loads(json.dumps(expected_data, default=str)) self.assertEqual({'Contact': user_data['Contact']}, expected_data) @@ -45,8 +49,7 @@ class TestRequestPersonalData(unittest.TestCase): frappe.db.sql("delete from `tabEmail Queue`") def create_user_if_not_exists(email, first_name = None): - if frappe.db.exists("User", email): - return + frappe.delete_doc_if_exists("User", email) frappe.get_doc({ "doctype": "User",