fix!: UX of supplier linking with supplier users on portal pages (#35836)

* fix: create and add Portal Users child table in Supplier/Customer

Issue #35772

* fix: modify the original permission check hook

* fix: auto-add role for portal users

* fix: added patch for auto-populating portal users

* fix: modify patch to fetch users correctly

* fix: remove unnecessary code for updating naming_series

* fix(UX): show portal user in list view

Also split columns to reduce whitespace.

* refactor: simpler role checking

* fix: consider parenttype while fetching portal user

* refactor: simpler code, rename variable

* test: supplier portal user can access their docs

* refactor: only add role if not added

* refactor: rename and move patch to supplier

* refactor: dont add role if no perm or existing doc

* fix: add role before save

* refactor: run query directly

* refactor: split patch and apply roles

- if role isn't present dont add portal user
- ignore failure as it's not critical

* test: fix permission creation for webform test

---------

Co-authored-by: Ankush Menat <ankush@frappe.io>

erpnext/buying/doctype/supplier/patches/migrate_supplier_portal_users.py

@@ -0,0 +1,56 @@
+import os
+
+import frappe
+
+in_ci = os.environ.get("CI")
+
+
+def execute():
+	try:
+		contacts = get_portal_user_contacts()
+		add_portal_users(contacts)
+	except Exception:
+		frappe.db.rollback()
+		frappe.log_error("Failed to migrate portal users")
+
+		if in_ci:  # TODO: better way to handle this.
+			raise
+
+
+def get_portal_user_contacts():
+	contact = frappe.qb.DocType("Contact")
+	dynamic_link = frappe.qb.DocType("Dynamic Link")
+
+	return (
+		frappe.qb.from_(contact)
+		.inner_join(dynamic_link)
+		.on(contact.name == dynamic_link.parent)
+		.select(
+			(dynamic_link.link_doctype).as_("doctype"),
+			(dynamic_link.link_name).as_("parent"),
+			(contact.email_id).as_("portal_user"),
+		)
+		.where(
+			(dynamic_link.parenttype == "Contact")
+			& (dynamic_link.link_doctype.isin(["Supplier", "Customer"]))
+		)
+	).run(as_dict=True)
+
+
+def add_portal_users(contacts):
+	for contact in contacts:
+		user = frappe.db.get_value("User", {"email": contact.portal_user}, "name")
+		if not user:
+			continue
+
+		roles = frappe.get_roles(user)
+		required_role = contact.doctype
+		if required_role not in roles:
+			continue
+
+		portal_user_doc = frappe.new_doc("Portal User")
+		portal_user_doc.parenttype = contact.doctype
+		portal_user_doc.parentfield = "portal_users"
+		portal_user_doc.parent = contact.parent
+		portal_user_doc.user = user
+		portal_user_doc.insert()

erpnext/buying/doctype/supplier/supplier.js

@@ -42,6 +42,14 @@ frappe.ui.form.on("Supplier", {
 				}
 			};
 		});
+
+		frm.set_query("user", "portal_users", function(doc) {
+			return {
+				filters: {
+					"ignore_user_type": true,
+				}
+			};
+		});
 	},
 
 	refresh: function (frm) {

erpnext/buying/doctype/supplier/supplier.json

@@ -68,7 +68,10 @@
   "on_hold",
   "hold_type",
   "column_break_59",
-  "release_date"
+  "release_date",
+  "portal_users_tab",
+  "portal_users",
+  "column_break_1mqv"
  ],
  "fields": [
   {
@@ -445,6 +448,21 @@
   {
    "fieldname": "column_break_59",
    "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "portal_users_tab",
+   "fieldtype": "Tab Break",
+   "label": "Portal Users"
+  },
+  {
+   "fieldname": "portal_users",
+   "fieldtype": "Table",
+   "label": "Supplier Portal Users",
+   "options": "Portal User"
+  },
+  {
+   "fieldname": "column_break_1mqv",
+   "fieldtype": "Column Break"
   }
  ],
  "icon": "fa fa-user",
@@ -457,7 +475,7 @@
    "link_fieldname": "party"
   }
  ],
- "modified": "2023-05-09 15:34:13.408932",
+ "modified": "2023-06-26 14:20:00.961554",
  "modified_by": "Administrator",
  "module": "Buying",
  "name": "Supplier",
@@ -489,7 +507,6 @@
    "read": 1,
    "report": 1,
    "role": "Purchase Master Manager",
-   "set_user_permissions": 1,
    "share": 1,
    "write": 1
   },

erpnext/buying/doctype/supplier/supplier.py

@@ -16,6 +16,7 @@ from erpnext.accounts.party import (  # noqa
 	get_timeline_data,
 	validate_party_accounts,
 )
+from erpnext.controllers.website_list_for_contact import add_role_for_portal_user
 from erpnext.utilities.transaction_base import TransactionBase
 
 
@@ -46,12 +47,35 @@ class Supplier(TransactionBase):
 			self.name = set_name_from_naming_options(frappe.get_meta(self.doctype).autoname, self)
 
 	def on_update(self):
-		if not self.naming_series:
-			self.naming_series = ""
-
 		self.create_primary_contact()
 		self.create_primary_address()
 
+	def add_role_for_user(self):
+		for portal_user in self.portal_users:
+			add_role_for_portal_user(portal_user, "Supplier")
+
+	def _add_supplier_role(self, portal_user):
+		if not portal_user.is_new():
+			return
+
+		user_doc = frappe.get_doc("User", portal_user.user)
+		roles = {r.role for r in user_doc.roles}
+
+		if "Supplier" in roles:
+			return
+
+		if "System Manager" not in frappe.get_roles():
+			frappe.msgprint(
+				_("Please add 'Supplier' role to user {0}.").format(portal_user.user),
+				alert=True,
+			)
+			return
+
+		user_doc.add_roles("Supplier")
+		frappe.msgprint(
+			_("Added Supplier Role to User {0}.").format(frappe.bold(user_doc.name)), alert=True
+		)
+
 	def validate(self):
 		self.flags.is_new_doc = self.is_new()
 
@@ -62,6 +86,7 @@ class Supplier(TransactionBase):
 
 		validate_party_accounts(self)
 		self.validate_internal_supplier()
+		self.add_role_for_user()
 
 	@frappe.whitelist()
 	def get_supplier_group_details(self):

erpnext/buying/doctype/supplier/test_supplier.py

@@ -7,6 +7,7 @@ from frappe.custom.doctype.property_setter.property_setter import make_property_
 from frappe.test_runner import make_test_records
 
 from erpnext.accounts.party import get_due_date
+from erpnext.controllers.website_list_for_contact import get_customers_suppliers
 from erpnext.exceptions import PartyDisabled
 
 test_dependencies = ["Payment Term", "Payment Terms Template"]
@@ -195,6 +196,9 @@ class TestSupplier(FrappeTestCase):
 def create_supplier(**args):
 	args = frappe._dict(args)
 
+	if not args.supplier_name:
+		args.supplier_name = frappe.generate_hash()
+
 	if frappe.db.exists("Supplier", args.supplier_name):
 		return frappe.get_doc("Supplier", args.supplier_name)
 
@@ -209,3 +213,25 @@ def create_supplier(**args):
 	).insert()
 
 	return doc
+
+
+class TestSupplierPortal(FrappeTestCase):
+	def test_portal_user_can_access_supplier_data(self):
+
+		supplier = create_supplier()
+
+		user = frappe.generate_hash() + "@example.com"
+		frappe.new_doc(
+			"User",
+			first_name="Supplier Portal User",
+			email=user,
+			send_welcome_email=False,
+		).insert()
+
+		supplier.append("portal_users", {"user": user})
+		supplier.save()
+
+		frappe.set_user(user)
+		_, suppliers = get_customers_suppliers("Purchase Order", user)
+
+		self.assertIn(supplier.name, suppliers)

erpnext/controllers/website_list_for_contact.py

@@ -232,22 +232,8 @@ def get_customers_suppliers(doctype, user):
 	has_supplier_field = meta.has_field("supplier")
 
 	if has_common(["Supplier", "Customer"], frappe.get_roles(user)):
-		contacts = frappe.db.sql(
-			"""
-			select
-				`tabContact`.email_id,
-				`tabDynamic Link`.link_doctype,
-				`tabDynamic Link`.link_name
-			from
-				`tabContact`, `tabDynamic Link`
-			where
-				`tabContact`.name=`tabDynamic Link`.parent and `tabContact`.email_id =%s
-			""",
-			user,
-			as_dict=1,
-		)
-		customers = [c.link_name for c in contacts if c.link_doctype == "Customer"]
-		suppliers = [c.link_name for c in contacts if c.link_doctype == "Supplier"]
+		suppliers = get_parents_for_user("Supplier")
+		customers = get_parents_for_user("Customer")
 	elif frappe.has_permission(doctype, "read", user=user):
 		customer_list = frappe.get_list("Customer")
 		customers = suppliers = [customer.name for customer in customer_list]
@@ -255,6 +241,17 @@ def get_customers_suppliers(doctype, user):
 	return customers if has_customer_field else None, suppliers if has_supplier_field else None
 
 
+def get_parents_for_user(parenttype: str) -> list[str]:
+	portal_user = frappe.qb.DocType("Portal User")
+
+	return (
+		frappe.qb.from_(portal_user)
+		.select(portal_user.parent)
+		.where(portal_user.user == frappe.session.user)
+		.where(portal_user.parenttype == parenttype)
+	).run(pluck="name")
+
+
 def has_website_permission(doc, ptype, user, verbose=False):
 	doctype = doc.doctype
 	customers, suppliers = get_customers_suppliers(doctype, user)
@@ -282,3 +279,28 @@ def get_customer_field_name(doctype):
 		return "party_name"
 	else:
 		return "customer"
+
+
+def add_role_for_portal_user(portal_user, role):
+	"""When a new portal user is added, give appropriate roles to user if
+	posssible, else warn user to add roles."""
+	if not portal_user.is_new():
+		return
+
+	user_doc = frappe.get_doc("User", portal_user.user)
+	roles = {r.role for r in user_doc.roles}
+
+	if role in roles:
+		return
+
+	if "System Manager" not in frappe.get_roles():
+		frappe.msgprint(
+			_("Please add {1} role to user {0}.").format(portal_user.user, role),
+			alert=True,
+		)
+		return
+
+	user_doc.add_roles(role)
+	frappe.msgprint(
+		_("Added {1} Role to User {0}.").format(frappe.bold(user_doc.name), role), alert=True
+	)

erpnext/patches.txt

@@ -339,4 +339,5 @@ execute:frappe.delete_doc('DocType', 'Cash Flow Mapper', ignore_missing=True)
 execute:frappe.delete_doc('DocType', 'Cash Flow Mapping Template', ignore_missing=True)
 execute:frappe.delete_doc('DocType', 'Cash Flow Mapping Accounts', ignore_missing=True)
 erpnext.patches.v14_0.cleanup_workspaces
-erpnext.patches.v14_0.set_report_in_process_SOA 
+erpnext.patches.v14_0.set_report_in_process_SOA
+erpnext.buying.doctype.supplier.patches.migrate_supplier_portal_users

erpnext/selling/doctype/customer/customer.js

@@ -63,6 +63,14 @@ frappe.ui.form.on("Customer", {
 				}
 			}
 		});
+
+		frm.set_query("user", "portal_users", function() {
+			return {
+				filters: {
+					"ignore_user_type": true,
+				}
+			};
+		});
 	},
 	customer_primary_address: function(frm){
 		if(frm.doc.customer_primary_address){

erpnext/selling/doctype/customer/customer.json

@@ -81,7 +81,9 @@
   "dn_required",
   "column_break_53",
   "is_frozen",
-  "disabled"
+  "disabled",
+  "portal_users_tab",
+  "portal_users"
  ],
  "fields": [
   {
@@ -555,6 +557,17 @@
   {
    "fieldname": "column_break_54",
    "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "portal_users_tab",
+   "fieldtype": "Tab Break",
+   "label": "Portal Users"
+  },
+  {
+   "fieldname": "portal_users",
+   "fieldtype": "Table",
+   "label": "Customer Portal Users",
+   "options": "Portal User"
   }
  ],
  "icon": "fa fa-user",
@@ -568,7 +581,7 @@
    "link_fieldname": "party"
   }
  ],
- "modified": "2023-05-09 15:38:40.255193",
+ "modified": "2023-06-22 13:21:10.678382",
  "modified_by": "Administrator",
  "module": "Selling",
  "name": "Customer",
@@ -607,7 +620,6 @@
    "read": 1,
    "report": 1,
    "role": "Sales Master Manager",
-   "set_user_permissions": 1,
    "share": 1,
    "write": 1
   },

erpnext/selling/doctype/customer/customer.py

@@ -22,6 +22,7 @@ from erpnext.accounts.party import (  # noqa
 	get_timeline_data,
 	validate_party_accounts,
 )
+from erpnext.controllers.website_list_for_contact import add_role_for_portal_user
 from erpnext.utilities.transaction_base import TransactionBase
 
 
@@ -82,6 +83,7 @@ class Customer(TransactionBase):
 		self.check_customer_group_change()
 		self.validate_default_bank_account()
 		self.validate_internal_customer()
+		self.add_role_for_user()
 
 		# set loyalty program tier
 		if frappe.db.exists("Customer", self.name):
@@ -170,6 +172,10 @@ class Customer(TransactionBase):
 
 		self.update_customer_groups()
 
+	def add_role_for_user(self):
+		for portal_user in self.portal_users:
+			add_role_for_portal_user(portal_user, "Customer")
+
 	def update_customer_groups(self):
 		ignore_doctypes = ["Lead", "Opportunity", "POS Profile", "Tax Rule", "Pricing Rule"]
 		if frappe.flags.customer_group_changed:

erpnext/tests/test_webform.py

@@ -3,18 +3,21 @@ import unittest
 import frappe
 
 from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
+from erpnext.buying.doctype.supplier.test_supplier import create_supplier
 
 
 class TestWebsite(unittest.TestCase):
 	def test_permission_for_custom_doctype(self):
 		create_user("Supplier 1", "supplier1@gmail.com")
 		create_user("Supplier 2", "supplier2@gmail.com")
-		create_supplier_with_contact(
-			"Supplier1", "All Supplier Groups", "Supplier 1", "supplier1@gmail.com"
-		)
-		create_supplier_with_contact(
-			"Supplier2", "All Supplier Groups", "Supplier 2", "supplier2@gmail.com"
-		)
+
+		supplier1 = create_supplier(supplier_name="Supplier1")
+		supplier2 = create_supplier(supplier_name="Supplier2")
+		supplier1.append("portal_users", {"user": "supplier1@gmail.com"})
+		supplier1.save()
+		supplier2.append("portal_users", {"user": "supplier2@gmail.com"})
+		supplier2.save()
+
 		po1 = create_purchase_order(supplier="Supplier1")
 		po2 = create_purchase_order(supplier="Supplier2")
 
@@ -61,21 +64,6 @@ def create_user(name, email):
 	).insert(ignore_if_duplicate=True)
 
 
-def create_supplier_with_contact(name, group, contact_name, contact_email):
-	supplier = frappe.get_doc(
-		{"doctype": "Supplier", "supplier_name": name, "supplier_group": group}
-	).insert(ignore_if_duplicate=True)
-
-	if not frappe.db.exists("Contact", contact_name + "-1-" + name):
-		new_contact = frappe.new_doc("Contact")
-		new_contact.first_name = contact_name
-		new_contact.is_primary_contact = (True,)
-		new_contact.append("links", {"link_doctype": "Supplier", "link_name": supplier.name})
-		new_contact.append("email_ids", {"email_id": contact_email, "is_primary": 1})
-
-		new_contact.insert(ignore_mandatory=True)
-
-
 def create_custom_doctype():
 	frappe.get_doc(
 		{

erpnext/utilities/doctype/portal_user/portal_user.json

@@ -0,0 +1,34 @@
+{
+ "actions": [],
+ "allow_rename": 1,
+ "creation": "2023-06-20 14:01:35.362233",
+ "doctype": "DocType",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "field_order": [
+  "user"
+ ],
+ "fields": [
+  {
+   "fieldname": "user",
+   "fieldtype": "Link",
+   "in_list_view": 1,
+   "label": "User",
+   "options": "User",
+   "reqd": 1,
+   "search_index": 1
+  }
+ ],
+ "index_web_pages_for_search": 1,
+ "istable": 1,
+ "links": [],
+ "modified": "2023-06-26 14:15:34.695605",
+ "modified_by": "Administrator",
+ "module": "Utilities",
+ "name": "Portal User",
+ "owner": "Administrator",
+ "permissions": [],
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "states": []
+}

erpnext/utilities/doctype/portal_user/portal_user.py

@@ -0,0 +1,9 @@
+# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+# import frappe
+from frappe.model.document import Document
+
+
+class PortalUser(Document):
+	pass