From 9abe68d6954ecd17db54ec5715f2bfb629bba38f Mon Sep 17 00:00:00 2001 From: A R R R Associates Date: Tue, 4 Aug 2026 22:59:18 +0530 Subject: [PATCH] Allow partners to add staff or branch managers --- app/modules/employees/service.py | 36 +++++++++++++++---- .../employees/templates/employees/form.html | 9 +++-- app/modules/employees/ui.py | 16 ++++++--- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/app/modules/employees/service.py b/app/modules/employees/service.py index 2578361..b9b9c34 100644 --- a/app/modules/employees/service.py +++ b/app/modules/employees/service.py @@ -292,12 +292,16 @@ def create_employee(db: Session, actor: User, scope: EmployeeScope, data: dict[s cleaned = _clean_payload(data) partner_staff_mode = bool(scope.is_partner and not scope.is_system_admin and not scope.is_firm_admin) - # Partners can create staff only in their own tenant and branch. Never trust - # tenant, branch or role values posted by the browser for this workflow. + # Partners can create Staff or Branch Manager users only in their own + # tenant and branch. Never trust tenant or branch values posted by the + # browser for this workflow, and validate the requested role server-side. if partner_staff_mode: cleaned["tenant_id"] = scope.tenant_id cleaned["branch_id"] = scope.branch_id or actor.branch_id - cleaned["employee_role"] = "Staff" + requested_role = (cleaned.get("employee_role") or "Staff").strip() + if requested_role not in {"Staff", "Branch Manager"}: + raise HTTPException(status_code=403, detail="Partners can create only Staff or Branch Manager users.") + cleaned["employee_role"] = requested_role tenant_id = int(cleaned.get("tenant_id") or scope.tenant_id) branch_id = int(cleaned.get("branch_id") or actor.branch_id) @@ -322,9 +326,10 @@ def create_employee(db: Session, actor: User, scope: EmployeeScope, data: dict[s raise HTTPException(status_code=400, detail="Selected user must belong to the employee tenant and branch.") if partner_staff_mode: linked_roles = _role_set(db, linked_user) - elevated_roles = {"System Admin", "Firm Admin", "Partner", "Branch Manager"} - if "Staff" not in linked_roles or linked_roles.intersection(elevated_roles): - raise HTTPException(status_code=403, detail="Partners can link only a Staff login user.") + allowed_roles = {"Staff", "Branch Manager"} + prohibited_roles = {"System Admin", "Firm Admin", "Partner"} + if not linked_roles.intersection(allowed_roles) or linked_roles.intersection(prohibited_roles): + raise HTTPException(status_code=403, detail="Partners can link only a Staff or Branch Manager login user.") user_id = linked_user.id elif cleaned.get("create_login_user"): login_user = create_login_user_for_employee( @@ -505,7 +510,24 @@ def link_employee_to_user(db: Session, actor: User, emp: Employee, user_id: int def list_reporting_managers(db: Session, scope: EmployeeScope) -> list[User]: - stmt = select(User).where(User.tenant_id == scope.tenant_id, User.deleted_at.is_(None), User.is_active.is_(True)) + """Return only active Partner and Branch Manager users in scope. + + Client and other non-employee login roles must never appear in the + Reporting Manager dropdown. + """ + stmt = ( + select(User) + .join(UserRole, UserRole.user_id == User.id) + .join(Role, Role.id == UserRole.role_id) + .where( + User.tenant_id == scope.tenant_id, + User.deleted_at.is_(None), + User.is_active.is_(True), + Role.is_active.is_(True), + Role.name.in_(("Partner", "Branch Manager")), + ) + .distinct() + ) if scope.branch_id is not None: stmt = stmt.where(User.branch_id == scope.branch_id) return db.execute(stmt.order_by(User.full_name, User.email)).scalars().all() diff --git a/app/modules/employees/templates/employees/form.html b/app/modules/employees/templates/employees/form.html index 97a328c..191b5d2 100644 --- a/app/modules/employees/templates/employees/form.html +++ b/app/modules/employees/templates/employees/form.html @@ -44,9 +44,12 @@
{% if partner_staff_mode %} - - -

Partners can create Staff users only.

+ +

The user will be created for your branch as Staff or Manager.

{% else %} {% endif %} diff --git a/app/modules/employees/ui.py b/app/modules/employees/ui.py index 22fd92b..e6f33a7 100644 --- a/app/modules/employees/ui.py +++ b/app/modules/employees/ui.py @@ -328,10 +328,17 @@ def _form_options(db, current_user, scope, *, include_user_id: int | None = None users = list_linkable_users(db, scope, include_user_id=include_user_id) partner_staff_mode = bool(scope.is_partner and not scope.is_system_admin and not scope.is_firm_admin) - # A Partner may onboard or link Staff users only. The service layer repeats - # this rule so a forged POST cannot bypass the form restriction. + # A Partner may onboard or link only Staff and Branch Manager users. The + # service layer repeats this rule so a forged POST cannot bypass the form. if partner_staff_mode: - users = [user for user in users if "Staff" in set(get_user_roles(db, user.id))] + allowed_roles = {"Staff", "Branch Manager"} + prohibited_roles = {"System Admin", "Firm Admin", "Partner"} + users = [ + user + for user in users + if set(get_user_roles(db, user.id)).intersection(allowed_roles) + and not set(get_user_roles(db, user.id)).intersection(prohibited_roles) + ] return { "tenants": visible_tenants(db, current_user), @@ -601,7 +608,8 @@ async def employee_create_submit(request: Request): scope = build_employee_scope(db, current_user, tenant_id=form.get("tenant_id"), branch_id=form.get("branch_id")) payload = _form_payload(form, include_context=True) if scope.is_partner and not scope.is_system_admin and not scope.is_firm_admin: - payload["employee_role"] = "Staff" + requested_role = (payload.get("employee_role") or "Staff").strip() + payload["employee_role"] = requested_role if requested_role in {"Staff", "Branch Manager"} else "Staff" payload["tenant_id"] = scope.tenant_id payload["branch_id"] = scope.branch_id or current_user.branch_id try: