fix(healthcare): Duplicate Contact error on add Patient (#27427)

* fix: duplicate contact error when linking existing Customer to Patient
validation for existing User email and mobile before creating user on Patient update

* test: patient - test contact, user creation

* fix: test_patient_contact clearing contact breaking other tests
sider issues

* fix: use db_set instead of set_value

* fix(test): overlapping appointments

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>

erpnext/healthcare/doctype/patient/patient.py

@@ -40,7 +40,7 @@ class Patient(Document):
 			frappe.db.set_value('Patient', self.name, 'status', 'Disabled')
 		else:
 			send_registration_sms(self)
-		self.reload() # self.notify_update()
+		self.reload()
 
 	def on_update(self):
 		if frappe.db.get_single_value('Healthcare Settings', 'link_customer_to_patient'):
@@ -93,7 +93,11 @@ class Patient(Document):
 			self.language = frappe.db.get_single_value('System Settings', 'language')
 
 	def create_website_user(self):
-		if self.email and not frappe.db.exists('User', self.email):
+		users = frappe.db.get_all('User', fields=['email', 'mobile_no'], or_filters={'email': self.email, 'mobile_no': self.mobile})
+		if users and users[0]:
+			frappe.throw(_("User exists with Email {}, Mobile {}<br>Please check email / mobile or disable 'Invite as User' to skip creating User")
+				.format(frappe.bold(users[0].email), frappe.bold(users[0].mobile_no)), frappe.DuplicateEntryError)
+
 		user = frappe.get_doc({
 			'doctype': 'User',
 			'first_name': self.first_name,
@@ -109,7 +113,7 @@ class Patient(Document):
 		user.enabled = True
 		user.send_welcome_email = True
 		user.add_roles('Patient')
-			frappe.db.set_value(self.doctype, self.name, 'user_id', user.name)
+		self.db_set('user_id', user.name)
 
 	def autoname(self):
 		patient_name_by = frappe.db.get_single_value('Healthcare Settings', 'patient_name_by')
@@ -159,10 +163,22 @@ class Patient(Document):
 			return {'invoice': sales_invoice.name}
 
 	def set_contact(self):
-		if frappe.db.exists('Dynamic Link', {'parenttype':'Contact', 'link_doctype':'Patient', 'link_name':self.name}):
+		contact = get_default_contact(self.doctype, self.name)
+
+		if contact:
 			old_doc = self.get_doc_before_save()
+			if not old_doc:
+				return
+
 			if old_doc.email != self.email or old_doc.mobile != self.mobile or old_doc.phone != self.phone:
-				self.update_contact()
+				self.update_contact(contact)
+		else:
+			if self.customer:
+				# customer contact exists, link patient
+				contact = get_default_contact('Customer', self.customer)
+
+			if contact:
+				self.update_contact(contact)
 			else:
 				self.reload()
 				if self.email or self.mobile or self.phone:
@@ -179,15 +195,14 @@ class Patient(Document):
 						contact.append('links', dict(link_doctype='Customer', link_name=self.customer))
 
 					contact.insert(ignore_permissions=True)
-				self.update_contact(contact) # update email, mobile and phone
+					self.update_contact(contact.name)
 
-	def update_contact(self, contact=None):
+	def update_contact(self, contact):
-		if not contact:
+		contact = frappe.get_doc('Contact', contact)
-			contact_name = get_default_contact(self.doctype, self.name)
+
-			if contact_name:
+		if not contact.has_link(self.doctype, self.name):
-				contact = frappe.get_doc('Contact', contact_name)
+			contact.append('links', dict(link_doctype=self.doctype, link_name=self.name))
 
-		if contact:
 		if self.email and self.email != contact.email_id:
 			for email in contact.email_ids:
 				email.is_primary = True if email.email_id == self.email else False
@@ -206,7 +221,7 @@ class Patient(Document):
 			contact.add_phone(self.phone, is_primary_phone=True)
 			contact.set_primary('phone')
 
-		contact.flags.ignore_validate = True # disable hook TODO: safe?
+		contact.flags.skip_patient_update = True
 		contact.save(ignore_permissions=True)
 
 

erpnext/healthcare/doctype/patient/test_patient.py

@@ -35,3 +35,40 @@ class TestPatient(unittest.TestCase):
 
 		settings.collect_registration_fee = 0
 		settings.save()
+
+	def test_patient_contact(self):
+		frappe.db.sql("""delete from `tabPatient` where name like '_Test Patient%'""")
+		frappe.db.sql("""delete from `tabCustomer` where name like '_Test Patient%'""")
+		frappe.db.sql("""delete from `tabContact` where name like'_Test Patient%'""")
+		frappe.db.sql("""delete from `tabDynamic Link` where parent like '_Test Patient%'""")
+
+		patient = create_patient(patient_name='_Test Patient Contact', email='test-patient@example.com', mobile='+91 0000000001')
+		customer = frappe.db.get_value('Patient', patient, 'customer')
+		self.assertTrue(customer)
+		self.assertTrue(frappe.db.exists('Dynamic Link', {'parenttype': 'Contact', 'link_doctype': 'Patient', 'link_name': patient}))
+		self.assertTrue(frappe.db.exists('Dynamic Link', {'parenttype': 'Contact', 'link_doctype': 'Customer', 'link_name': customer}))
+
+		# a second patient linking with same customer
+		new_patient = create_patient(email='test-patient@example.com', mobile='+91 0000000009', customer=customer)
+		self.assertTrue(frappe.db.exists('Dynamic Link', {'parenttype': 'Contact', 'link_doctype': 'Patient', 'link_name': new_patient}))
+		self.assertTrue(frappe.db.exists('Dynamic Link', {'parenttype': 'Contact', 'link_doctype': 'Customer', 'link_name': customer}))
+
+	def test_patient_user(self):
+		frappe.db.sql("""delete from `tabUser` where email='test-patient-user@example.com'""")
+		frappe.db.sql("""delete from `tabDynamic Link` where parent like '_Test Patient%'""")
+		frappe.db.sql("""delete from `tabPatient` where name like '_Test Patient%'""")
+
+		patient = create_patient(patient_name='_Test Patient User', email='test-patient-user@example.com', mobile='+91 0000000009', create_user=True)
+		user = frappe.db.get_value('Patient', patient, 'user_id')
+		self.assertTrue(frappe.db.exists('User', user))
+
+		new_patient = frappe.get_doc({
+			'doctype': 'Patient',
+			'first_name': '_Test Patient Duplicate User',
+			'sex': 'Male',
+			'email': 'test-patient-user@example.com',
+			'mobile': '+91 0000000009',
+			'invite_user': 1
+		})
+
+		self.assertRaises(frappe.exceptions.DuplicateEntryError, new_patient.insert)

erpnext/healthcare/doctype/patient_appointment/test_patient_appointment.py

@@ -307,14 +307,18 @@ def create_healthcare_docs(id=0):
 	return patient, practitioner
 
 
-def create_patient(id=0):
+def create_patient(id=0, patient_name=None, email=None, mobile=None, customer=None, create_user=False):
 	if frappe.db.exists('Patient', {'firstname':f'_Test Patient {str(id)}'}):
 		patient = frappe.db.get_value('Patient', {'first_name': f'_Test Patient {str(id)}'}, ['name'])
 		return patient
 
 	patient = frappe.new_doc('Patient')
-	patient.first_name = f'_Test Patient {str(id)}'
+	patient.first_name = patient_name if patient_name else f'_Test Patient {str(id)}'
 	patient.sex = 'Female'
+	patient.mobile = mobile
+	patient.email = email
+	patient.customer = customer
+	patient.invite_user = create_user
 	patient.save(ignore_permissions=True)
 
 	return patient.name

erpnext/healthcare/doctype/patient_medical_record/test_patient_medical_record.py

@@ -6,7 +6,7 @@ from __future__ import unicode_literals
 import unittest
 
 import frappe
-from frappe.utils import nowdate
+from frappe.utils import add_days, nowdate
 
 from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile
 from erpnext.healthcare.doctype.patient_appointment.test_patient_appointment import (
@@ -38,7 +38,7 @@ class TestPatientMedicalRecord(unittest.TestCase):
 		medical_rec = frappe.db.exists('Patient Medical Record', {'status': 'Open', 'reference_name': vital_signs.name})
 		self.assertTrue(medical_rec)
 
-		appointment = create_appointment(patient, practitioner, nowdate(), invoice=1, procedure_template=1)
+		appointment = create_appointment(patient, practitioner, add_days(nowdate(), 1), invoice=1, procedure_template=1)
 		procedure = create_procedure(appointment)
 		procedure.start_procedure()
 		procedure.complete_procedure()

erpnext/healthcare/utils.py

@@ -776,7 +776,7 @@ def update_patient_email_and_phone_numbers(contact, method):
 	Hook validate Contact
 	Update linked Patients' primary mobile and phone numbers
 	'''
-	if 'Healthcare' not in frappe.get_active_domains():
+	if 'Healthcare' not in frappe.get_active_domains() or contact.flags.skip_patient_update:
 		return
 
 	if contact.is_primary_contact and (contact.email_id or contact.mobile_no or contact.phone):
@@ -784,9 +784,15 @@ def update_patient_email_and_phone_numbers(contact, method):
 
 		for link in patient_links:
 			contact_details = frappe.db.get_value('Patient', link.get('link_name'), ['email', 'mobile', 'phone'], as_dict=1)
+
+			new_contact_details = {}
+
 			if contact.email_id and contact.email_id != contact_details.get('email'):
-				frappe.db.set_value('Patient', link.get('link_name'), 'email', contact.email_id)
+				new_contact_details.update({'email': contact.email_id})
 			if contact.mobile_no and contact.mobile_no != contact_details.get('mobile'):
-				frappe.db.set_value('Patient', link.get('link_name'), 'mobile', contact.mobile_no)
+				new_contact_details.update({'mobile': contact.mobile_no})
 			if contact.phone and contact.phone != contact_details.get('phone'):
-				frappe.db.set_value('Patient', link.get('link_name'), 'phone', contact.phone)
+				new_contact_details.update({'phone': contact.phone})
+
+			if new_contact_details:
+				frappe.db.set_value('Patient', link.get('link_name'), new_contact_details)