Merge pull request #26085 from anupamvs/lead-cleanup

refactor: lead

erpnext/crm/doctype/appointment/test_appointment.py

@@ -9,7 +9,7 @@ import datetime
 
 
 def create_test_lead():
-    test_lead = frappe.db.exists({'doctype': 'Lead', 'lead_name': 'Test Lead'})
+    test_lead = frappe.db.exists({'doctype': 'Lead', 'email_id':'test@example.com'})
     if test_lead:
         return frappe.get_doc('Lead', test_lead[0][0])
     test_lead = frappe.get_doc({

erpnext/crm/doctype/lead/lead.js

@@ -12,7 +12,8 @@ erpnext.LeadController = class LeadController extends frappe.ui.form.Controller
 			'Opportunity': this.make_opportunity
 		};
 
-		this.frm.toggle_reqd("lead_name", !this.frm.doc.organization_lead);
+		// For avoiding integration issues.
+		this.frm.set_df_property('first_name', 'reqd', true);
 	}
 
 	onload () {
@@ -42,6 +43,7 @@ erpnext.LeadController = class LeadController extends frappe.ui.form.Controller
 
 		if (!this.frm.is_new()) {
 			frappe.contacts.render_address_and_contact(this.frm);
+			cur_frm.trigger('render_contact_day_html');
 		} else {
 			frappe.contacts.clear_address_and_contact(this.frm);
 		}
@@ -68,13 +70,8 @@ erpnext.LeadController = class LeadController extends frappe.ui.form.Controller
 		})
 	}
 
-	organization_lead () {
-		this.frm.toggle_reqd("lead_name", !this.frm.doc.organization_lead);
-		this.frm.toggle_reqd("company_name", this.frm.doc.organization_lead);
-	}
-
 	company_name () {
-		if (this.frm.doc.organization_lead && !this.frm.doc.lead_name) {
+		if (!this.frm.doc.lead_name) {
 			this.frm.set_value("lead_name", this.frm.doc.company_name);
 		}
 	}
@@ -86,6 +83,19 @@ erpnext.LeadController = class LeadController extends frappe.ui.form.Controller
 			this.frm.set_value("ends_on", d.format(frappe.defaultDatetimeFormat));
 		}
 	}
+
+	render_contact_day_html() {
+		if (cur_frm.doc.contact_date) {
+			let contact_date = frappe.datetime.obj_to_str(cur_frm.doc.contact_date);
+			let diff_days = frappe.datetime.get_day_diff(contact_date, frappe.datetime.get_today());
+			let color = diff_days > 0 ? "orange" : "green";
+			let message = diff_days > 0 ? __("Next Contact Date") : __("Last Contact Date");
+			let html = `<div class="col-xs-12">
+						<span class="indicator whitespace-nowrap ${color}"><span> ${message} : ${frappe.datetime.global_date_format(contact_date)}</span></span>
+					</div>` ;
+			cur_frm.dashboard.set_headline_alert(html);
+		}
+	}
 };
 
 extend_cscript(cur_frm.cscript, new erpnext.LeadController({ frm: cur_frm }));

erpnext/crm/doctype/lead/lead.json

@@ -9,71 +9,70 @@
  "email_append_to": 1,
  "engine": "InnoDB",
  "field_order": [
-  "organization_lead",
   "lead_details",
   "naming_series",
-  "lead_name",
-  "company_name",
-  "email_id",
-  "col_break123",
-  "lead_owner",
-  "status",
   "salutation",
+  "first_name",
+  "middle_name",
+  "last_name",
+  "lead_name",
+  "col_break123",
+  "status",
+  "company_name",
   "designation",
   "gender",
-  "source",
+  "contact_details_section",
-  "customer",
+  "email_id",
-  "campaign_name",
+  "mobile_no",
-  "image",
+  "whatsapp_no",
-  "section_break_12",
+  "column_break_16",
-  "contact_by",
+  "phone",
-  "column_break_14",
+  "phone_ext",
-  "contact_date",
+  "additional_information_section",
-  "ends_on",
+  "no_of_employees",
-  "notes_section",
+  "industry",
-  "notes",
+  "market_segment",
-  "address_info",
+  "column_break_22",
+  "fax",
+  "website",
+  "type",
+  "request_type",
+  "address_section",
   "address_html",
-  "address_type",
-  "address_title",
-  "address_line1",
-  "address_line2",
   "city",
+  "pincode",
   "county",
   "column_break2",
   "contact_html",
   "state",
   "country",
-  "pincode",
+  "section_break_12",
-  "contact_section",
+  "lead_owner",
-  "phone",
+  "ends_on",
-  "mobile_no",
+  "column_break_14",
-  "fax",
+  "contact_by",
-  "website",
+  "contact_date",
-  "more_info",
+  "lead_source_details_section",
-  "type",
-  "market_segment",
-  "industry",
-  "request_type",
-  "column_break3",
   "company",
   "territory",
   "language",
+  "column_break_50",
+  "source",
+  "campaign_name",
   "unsubscribed",
   "blog_subscriber",
+  "notes_section",
+  "notes",
+  "other_information_section",
+  "customer",
+  "image",
   "title"
  ],
  "fields": [
-  {
-   "default": "0",
-   "fieldname": "organization_lead",
-   "fieldtype": "Check",
-   "label": "Lead is an Organization",
-   "set_only_once": 1
-  },
   {
    "fieldname": "lead_details",
    "fieldtype": "Section Break",
+   "label": "Lead Details",
    "options": "fa fa-user"
   },
   {
@@ -90,16 +89,19 @@
    "fieldname": "lead_name",
    "fieldtype": "Data",
    "in_global_search": 1,
-   "label": "Person Name",
+   "label": "Full Name",
    "oldfieldname": "lead_name",
    "oldfieldtype": "Data",
+   "read_only": 1,
    "search_index": 1
   },
   {
    "fieldname": "company_name",
    "fieldtype": "Data",
    "in_list_view": 1,
+   "in_standard_filter": 1,
    "label": "Organization Name",
+   "mandatory_depends_on": "eval: !(doc.first_name)",
    "oldfieldname": "company_name",
    "oldfieldtype": "Data"
   },
@@ -121,7 +123,6 @@
    "default": "__user",
    "fieldname": "lead_owner",
    "fieldtype": "Link",
-   "in_list_view": 1,
    "label": "Lead Owner",
    "oldfieldname": "lead_owner",
    "oldfieldtype": "Link",
@@ -143,7 +144,6 @@
    "search_index": 1
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "salutation",
    "fieldtype": "Link",
    "label": "Salutation",
@@ -241,46 +241,22 @@
    "read_only": 1
   },
   {
-   "depends_on": "eval: doc.__islocal",
-   "description": "Home, Work, etc.",
-   "fieldname": "address_title",
-   "fieldtype": "Data",
-   "label": "Address Title"
-  },
-  {
-   "depends_on": "eval: doc.__islocal",
-   "fieldname": "address_line1",
-   "fieldtype": "Data",
-   "label": "Address Line 1",
-   "mandatory_depends_on": "eval: doc.address_title && doc.address_type"
-  },
-  {
-   "depends_on": "eval: doc.__islocal",
-   "fieldname": "address_line2",
-   "fieldtype": "Data",
-   "label": "Address Line 2"
-  },
-  {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "city",
    "fieldtype": "Data",
    "label": "City/Town",
    "mandatory_depends_on": "eval: doc.address_title && doc.address_type"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "county",
    "fieldtype": "Data",
    "label": "County"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "state",
    "fieldtype": "Data",
    "label": "State"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "country",
    "fieldtype": "Link",
    "label": "Country",
@@ -288,7 +264,6 @@
    "options": "Country"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "pincode",
    "fieldtype": "Data",
    "label": "Postal Code"
@@ -304,7 +279,6 @@
    "read_only": 1
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "phone",
    "fieldtype": "Data",
    "label": "Phone",
@@ -313,7 +287,6 @@
    "options": "Phone"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "mobile_no",
    "fieldtype": "Data",
    "label": "Mobile No.",
@@ -322,21 +295,12 @@
    "options": "Phone"
   },
   {
-   "depends_on": "eval: doc.__islocal",
    "fieldname": "fax",
    "fieldtype": "Data",
    "label": "Fax",
    "oldfieldname": "fax",
    "oldfieldtype": "Data"
   },
-  {
-   "collapsible": 1,
-   "fieldname": "more_info",
-   "fieldtype": "Section Break",
-   "label": "More Information",
-   "oldfieldtype": "Section Break",
-   "options": "fa fa-file-text"
-  },
   {
    "fieldname": "type",
    "fieldtype": "Select",
@@ -369,12 +333,6 @@
    "oldfieldtype": "Select",
    "options": "\nProduct Enquiry\nRequest for Information\nSuggestions\nOther"
   },
-  {
-   "fieldname": "column_break3",
-   "fieldtype": "Column Break",
-   "oldfieldtype": "Column Break",
-   "width": "50%"
-  },
   {
    "fieldname": "company",
    "fieldtype": "Link",
@@ -389,11 +347,14 @@
    "fieldtype": "Data",
    "label": "Website",
    "oldfieldname": "website",
-   "oldfieldtype": "Data"
+   "oldfieldtype": "Data",
+   "options": "URL"
   },
   {
    "fieldname": "territory",
    "fieldtype": "Link",
+   "in_list_view": 1,
+   "in_standard_filter": 1,
    "label": "Territory",
    "oldfieldname": "territory",
    "oldfieldtype": "Link",
@@ -422,45 +383,95 @@
   {
    "fieldname": "designation",
    "fieldtype": "Link",
+   "in_list_view": 1,
+   "in_standard_filter": 1,
    "label": "Designation",
    "options": "Designation"
   },
-  {
-   "collapsible": 1,
-   "collapsible_depends_on": "eval: doc.__islocal",
-   "fieldname": "address_info",
-   "fieldtype": "Section Break",
-   "label": "Address & Contact",
-   "oldfieldtype": "Column Break",
-   "options": "fa fa-map-marker"
-  },
-  {
-   "collapsible": 1,
-   "collapsible_depends_on": "eval: doc.__islocal",
-   "fieldname": "contact_section",
-   "fieldtype": "Section Break",
-   "label": "Contact"
-  },
-  {
-   "default": "Billing",
-   "depends_on": "eval: doc.__islocal",
-   "fieldname": "address_type",
-   "fieldtype": "Select",
-   "label": "Address Type",
-   "options": "Billing\nShipping\nOffice\nPersonal\nPlant\nPostal\nShop\nSubsidiary\nWarehouse\nCurrent\nPermanent\nOther"
-  },
   {
    "fieldname": "language",
    "fieldtype": "Link",
    "label": "Print Language",
    "options": "Language"
+  },
+  {
+   "fieldname": "first_name",
+   "fieldtype": "Data",
+   "label": "First Name",
+   "mandatory_depends_on": "eval: !(doc.company_name)"
+  },
+  {
+   "fieldname": "middle_name",
+   "fieldtype": "Data",
+   "label": "Middle Name"
+  },
+  {
+   "fieldname": "last_name",
+   "fieldtype": "Data",
+   "label": "Last Name"
+  },
+  {
+   "collapsible": 1,
+   "fieldname": "additional_information_section",
+   "fieldtype": "Section Break",
+   "label": "Additional Information"
+  },
+  {
+   "fieldname": "no_of_employees",
+   "fieldtype": "Int",
+   "label": "No. of Employees"
+  },
+  {
+   "fieldname": "column_break_22",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "whatsapp_no",
+   "fieldtype": "Data",
+   "label": "WhatsApp No.",
+   "options": "Phone"
+  },
+  {
+   "collapsible": 1,
+   "depends_on": "eval: !doc.__islocal",
+   "fieldname": "address_section",
+   "fieldtype": "Section Break",
+   "label": "Address"
+  },
+  {
+   "fieldname": "lead_source_details_section",
+   "fieldtype": "Section Break",
+   "label": "Lead Source Details"
+  },
+  {
+   "fieldname": "column_break_50",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "other_information_section",
+   "fieldtype": "Section Break",
+   "label": "Other Information"
+  },
+  {
+   "fieldname": "contact_details_section",
+   "fieldtype": "Section Break",
+   "label": "Contact Details"
+  },
+  {
+   "fieldname": "column_break_16",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "phone_ext",
+   "fieldtype": "Data",
+   "label": "Phone Ext."
   }
  ],
  "icon": "fa fa-user",
  "idx": 5,
  "image_field": "image",
  "links": [],
- "modified": "2021-01-06 19:39:58.748978",
+ "modified": "2021-08-04 00:24:57.208590",
  "modified_by": "Administrator",
  "module": "CRM",
  "name": "Lead",

erpnext/crm/doctype/lead/lead.py

@@ -21,26 +21,24 @@ class Lead(SellingController):
 		self.get("__onload").is_customer = customer
 		load_address_and_contact(self)
 
-	def before_insert(self):
-		if self.address_title and self.address_type:
-			self.address_doc = self.create_address()
-		self.contact_doc = self.create_contact()
-
-	def after_insert(self):
-		self.update_links()
-
 	def validate(self):
+		self.set_full_name()
 		self.set_lead_name()
 		self.set_title()
+		self.set_status()
+		self.check_email_id_is_unique()
+		self.validate_email_id()
+		self.validate_contact_date()
 		self._prev = frappe._dict({
 			"contact_date": frappe.db.get_value("Lead", self.name, "contact_date") if (not cint(self.is_new())) else None,
 			"ends_on": frappe.db.get_value("Lead", self.name, "ends_on") if (not cint(self.is_new())) else None,
 			"contact_by": frappe.db.get_value("Lead", self.name, "contact_by") if (not cint(self.is_new())) else None,
 		})
 		
-		self.set_status()
+	def set_full_name(self):
-		self.check_email_id_is_unique()
+		self.lead_name = " ".join(filter(None, [self.first_name, self.middle_name, self.last_name]))
 
+	def validate_email_id(self):
 		if self.email_id:
 			if not self.flags.ignore_email_validation:
 				validate_email_address(self.email_id, throw=True)
@@ -54,6 +52,7 @@ class Lead(SellingController):
 			if self.is_new() or not self.image:
 				self.image = has_gravatar(self.email_id)
 
+	def validate_contact_date(self):
 		if self.contact_date and getdate(self.contact_date) < getdate(nowdate()):
 			frappe.throw(_("Next Contact Date cannot be in the past"))
 
@@ -64,6 +63,22 @@ class Lead(SellingController):
 	def on_update(self):
 		self.add_calendar_event()
 
+	def before_insert(self):
+		self.contact_doc = self.create_contact()
+
+	def after_insert(self):
+		self.update_links()
+
+	def update_links(self):
+		# update contact links
+		if self.contact_doc:
+			self.contact_doc.append("links", {
+				"link_doctype": "Lead",
+				"link_name": self.name,
+				"link_title": self.lead_name
+			})
+			self.contact_doc.save()
+
 	def add_calendar_event(self, opts=None, force=False):
 		super(Lead, self).add_calendar_event({
 			"owner": self.lead_owner,
@@ -86,8 +101,26 @@ class Lead(SellingController):
 	def on_trash(self):
 		frappe.db.sql("""update `tabIssue` set lead='' where lead=%s""", self.name)
 
+		self.unlink_dynamic_links()
 		self.delete_events()
 
+	def unlink_dynamic_links(self):
+		links = frappe.get_all('Dynamic Link', filters={'link_doctype': self.doctype, 'link_name': self.name}, fields=['parent', 'parenttype'])
+
+		for link in links:
+			linked_doc = frappe.get_doc(link['parenttype'], link['parent'])
+
+			if len(linked_doc.get('links')) == 1:
+				linked_doc.delete(ignore_permissions=True)
+			else:
+				to_remove = None
+				for d in linked_doc.get('links'):
+					if d.link_doctype == self.doctype and d.link_name == self.name:
+						to_remove = d
+				if to_remove:
+					linked_doc.remove(to_remove)
+					linked_doc.save(ignore_permissions=True)
+
 	def has_customer(self):
 		return frappe.db.get_value("Customer", {"lead_name": self.name})
 
@@ -99,7 +132,6 @@ class Lead(SellingController):
 			"party_name": self.name,
 			"docstatus": 1,
 			"status": ["!=", "Lost"]
-
 		})
 
 	def has_lost_quotation(self):
@@ -120,40 +152,17 @@ class Lead(SellingController):
 				self.lead_name = self.email_id.split("@")[0]
 
 	def set_title(self):
-		if self.organization_lead:
+		self.title = self.company_name or self.lead_name
-			self.title = self.company_name
-		else:
-			self.title = self.lead_name
-
-	def create_address(self):
-		address_fields = ["address_type", "address_title", "address_line1", "address_line2",
-			"city", "county", "state", "country", "pincode"]
-		info_fields = ["email_id", "phone", "fax"]
-
-		# do not create an address if no fields are available,
-		# skipping country since the system auto-sets it from system defaults
-		address = frappe.new_doc("Address")
-
-		address.update({addr_field: self.get(addr_field) for addr_field in address_fields})
-		address.update({info_field: self.get(info_field) for info_field in info_fields})
-		address.insert()
-
-		return address
 
 	def create_contact(self):
 		if not self.lead_name:
+			self.set_full_name()
 			self.set_lead_name()
 
-		names = self.lead_name.strip().split(" ")
-		if len(names) > 1:
-			first_name, last_name = names[0], " ".join(names[1:])
-		else:
-			first_name, last_name = self.lead_name, None
-
 		contact = frappe.new_doc("Contact")
 		contact.update({
-			"first_name": first_name,
+			"first_name": self.first_name or self.lead_name,
-			"last_name": last_name,
+			"last_name": self.last_name,
 			"salutation": self.salutation,
 			"gender": self.gender,
 			"designation": self.designation,
@@ -181,25 +190,6 @@ class Lead(SellingController):
 
 		return contact
 
-	def update_links(self):
-		# update address links
-		if hasattr(self, 'address_doc'):
-			self.address_doc.append("links", {
-				"link_doctype": "Lead",
-				"link_name": self.name,
-				"link_title": self.lead_name
-			})
-			self.address_doc.save()
-
-		# update contact links
-		if self.contact_doc:
-			self.contact_doc.append("links", {
-				"link_doctype": "Lead",
-				"link_name": self.name,
-				"link_title": self.lead_name
-			})
-			self.contact_doc.save()
-
 @frappe.whitelist()
 def make_customer(source_name, target_doc=None):
 	return _make_customer(source_name, target_doc)

erpnext/crm/doctype/lead/test_lead.py

@@ -4,6 +4,7 @@
 from __future__ import unicode_literals
 
 import frappe
+from frappe.utils import random_string
 import unittest
 
 test_records = frappe.get_test_records('Lead')
@@ -32,3 +33,53 @@ class TestLead(unittest.TestCase):
 		customer.company = "_Test Company"
 		customer.customer_group = "_Test Customer Group"
 		customer.insert()
+
+	def test_create_lead_and_unlinking_dynamic_links(self):
+		lead_doc = make_lead(first_name = "Lorem", last_name="Ipsum", email_id="lorem_ipsum@example.com")
+		lead_doc_1 = make_lead()
+		frappe.get_doc({
+			"doctype": "Address",
+			"address_type": "Billing",
+			"city": "Mumbai",
+			"address_line1": "Vidya Vihar West",
+			"country": "India",
+			"links": [{
+				"link_doctype": "Lead",
+				"link_name": lead_doc.name
+			}]
+		}).insert()
+
+		address_1 = frappe.get_doc({
+			"doctype": "Address",
+			"address_type": "Billing",
+			"address_line1": "Baner",
+			"city": "Pune",
+			"country": "India",
+			"links": [
+				{
+					"link_doctype": "Lead",
+					"link_name": lead_doc.name
+				},
+				{
+					"link_doctype": "Lead",
+					"link_name": lead_doc_1.name
+				}
+			]
+		}).insert()
+
+		lead_doc.delete()
+		address_1.reload()
+		self.assertEqual(frappe.db.exists("Lead",lead_doc.name), None)
+		self.assertEqual(len(address_1.get('links')), 1)
+
+def make_lead(**args):
+	args = frappe._dict(args)
+
+	lead_doc = frappe.get_doc({
+		"doctype": "Lead",
+		"first_name": args.first_name or "_Test",
+		"last_name": args.last_name or "Lead",
+		"email_id": args.email_id or "new_lead_{}@example.com".format(random_string(5)),
+	}).insert()
+
+	return lead_doc

erpnext/crm/doctype/lead/test_records.json

@@ -27,7 +27,6 @@
 {
  "doctype": "Lead",
  "email_id": "test_lead4@example.com",
- "organization_lead": 1,
  "lead_name": "_Test Lead 4",
  "company_name": "_Test Lead 4",
  "status": "Open"

erpnext/crm/doctype/lead/tests/test_lead_organization.js

@@ -9,7 +9,6 @@ QUnit.test("test: lead", function (assert) {
 		() => frappe.set_route("List", "Lead"),
 		() => frappe.new_doc("Lead"),
 		() => frappe.timeout(1),
-		() => cur_frm.set_value("organization_lead", "1"),
 		() => cur_frm.set_value("company_name", lead_name),
 		() => cur_frm.save(),
 		() => frappe.timeout(1),

erpnext/selling/doctype/customer/customer.py

@@ -176,12 +176,12 @@ class Customer(TransactionBase):
 					address.append('links', dict(link_doctype='Customer', link_name=self.name))
 					address.save(ignore_permissions=self.flags.ignore_permissions)
 
-			lead = frappe.db.get_value("Lead", self.lead_name, ["organization_lead", "lead_name", "email_id", "phone", "mobile_no", "gender", "salutation"], as_dict=True)
+			lead = frappe.db.get_value("Lead", self.lead_name, ["company_name", "lead_name", "email_id", "phone", "mobile_no", "gender", "salutation"], as_dict=True)
 
 			if not lead.lead_name:
 				frappe.throw(_("Please mention the Lead Name in Lead {0}").format(self.lead_name))
 
-			if lead.organization_lead:
+			if lead.company_name:
 				contact_names = frappe.get_all('Dynamic Link', filters={
 									"parenttype":"Contact",
 									"link_doctype":"Lead",