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>
diff --git a/erpnext/buying/doctype/supplier/patches/__init__.py b/erpnext/buying/doctype/supplier/patches/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/erpnext/buying/doctype/supplier/patches/__init__.py
diff --git a/erpnext/buying/doctype/supplier/patches/migrate_supplier_portal_users.py b/erpnext/buying/doctype/supplier/patches/migrate_supplier_portal_users.py
new file mode 100644
index 0000000..5834952
--- /dev/null
+++ b/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()
diff --git a/erpnext/buying/doctype/supplier/supplier.js b/erpnext/buying/doctype/supplier/supplier.js
index 1ae6f03..a536578 100644
--- a/erpnext/buying/doctype/supplier/supplier.js
+++ b/erpnext/buying/doctype/supplier/supplier.js
@@ -42,6 +42,14 @@
}
};
});
+
+ frm.set_query("user", "portal_users", function(doc) {
+ return {
+ filters: {
+ "ignore_user_type": true,
+ }
+ };
+ });
},
refresh: function (frm) {
diff --git a/erpnext/buying/doctype/supplier/supplier.json b/erpnext/buying/doctype/supplier/supplier.json
index f009789..b3b6185 100644
--- a/erpnext/buying/doctype/supplier/supplier.json
+++ b/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
},
diff --git a/erpnext/buying/doctype/supplier/supplier.py b/erpnext/buying/doctype/supplier/supplier.py
index 01b5c8f..31bf439 100644
--- a/erpnext/buying/doctype/supplier/supplier.py
+++ b/erpnext/buying/doctype/supplier/supplier.py
@@ -16,6 +16,7 @@
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 @@
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 @@
validate_party_accounts(self)
self.validate_internal_supplier()
+ self.add_role_for_user()
@frappe.whitelist()
def get_supplier_group_details(self):
diff --git a/erpnext/buying/doctype/supplier/test_supplier.py b/erpnext/buying/doctype/supplier/test_supplier.py
index 7a205ac..7be1d83 100644
--- a/erpnext/buying/doctype/supplier/test_supplier.py
+++ b/erpnext/buying/doctype/supplier/test_supplier.py
@@ -7,6 +7,7 @@
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 @@
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 @@
).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)
diff --git a/erpnext/controllers/website_list_for_contact.py b/erpnext/controllers/website_list_for_contact.py
index 7c3c387..642722a 100644
--- a/erpnext/controllers/website_list_for_contact.py
+++ b/erpnext/controllers/website_list_for_contact.py
@@ -232,22 +232,8 @@
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 @@
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 @@
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
+ )
diff --git a/erpnext/patches.txt b/erpnext/patches.txt
index 8c0fa6b..fe6346e 100644
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -339,4 +339,5 @@
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
diff --git a/erpnext/selling/doctype/customer/customer.js b/erpnext/selling/doctype/customer/customer.js
index b53f339..3a446e1 100644
--- a/erpnext/selling/doctype/customer/customer.js
+++ b/erpnext/selling/doctype/customer/customer.js
@@ -63,6 +63,14 @@
}
}
});
+
+ frm.set_query("user", "portal_users", function() {
+ return {
+ filters: {
+ "ignore_user_type": true,
+ }
+ };
+ });
},
customer_primary_address: function(frm){
if(frm.doc.customer_primary_address){
diff --git a/erpnext/selling/doctype/customer/customer.json b/erpnext/selling/doctype/customer/customer.json
index 72a1594..edfe005 100644
--- a/erpnext/selling/doctype/customer/customer.json
+++ b/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
},
diff --git a/erpnext/selling/doctype/customer/customer.py b/erpnext/selling/doctype/customer/customer.py
index 6367e3c..555db59 100644
--- a/erpnext/selling/doctype/customer/customer.py
+++ b/erpnext/selling/doctype/customer/customer.py
@@ -22,6 +22,7 @@
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 @@
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 @@
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:
diff --git a/erpnext/tests/test_webform.py b/erpnext/tests/test_webform.py
index 202467b..af50a05 100644
--- a/erpnext/tests/test_webform.py
+++ b/erpnext/tests/test_webform.py
@@ -3,18 +3,21 @@
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 @@
).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(
{
diff --git a/erpnext/utilities/doctype/portal_user/__init__.py b/erpnext/utilities/doctype/portal_user/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/erpnext/utilities/doctype/portal_user/__init__.py
diff --git a/erpnext/utilities/doctype/portal_user/portal_user.json b/erpnext/utilities/doctype/portal_user/portal_user.json
new file mode 100644
index 0000000..361166c
--- /dev/null
+++ b/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": []
+}
\ No newline at end of file
diff --git a/erpnext/utilities/doctype/portal_user/portal_user.py b/erpnext/utilities/doctype/portal_user/portal_user.py
new file mode 100644
index 0000000..2e0064d
--- /dev/null
+++ b/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