diff --git a/app/modules/client_groups/service.py b/app/modules/client_groups/service.py index e868f8b..145f196 100644 --- a/app/modules/client_groups/service.py +++ b/app/modules/client_groups/service.py @@ -1,10 +1,11 @@ from __future__ import annotations -from sqlalchemy import func, select +from sqlalchemy import exists, func, or_, select from sqlalchemy.orm import Session from app.modules.client_groups.models import ClientGroup from app.modules.clients.models import Client +from app.modules.services.models import ClientServiceSubscription GROUP_TYPES = ("Family", "Business Group", "Promoter Group", "Trust Group", "Common Management", "Other") @@ -13,7 +14,7 @@ def normalise_group_code(value: str | None) -> str: return (value or "").strip().upper() -def list_groups(db: Session, *, tenant_id: int, include_inactive: bool = False): +def list_groups(db: Session, *, tenant_id: int, include_inactive: bool = False, viewer_partner_id: int | None = None): stmt = ( select(ClientGroup, func.count(Client.id).label("client_count")) .outerjoin(Client, Client.client_group_id == ClientGroup.id) @@ -21,6 +22,14 @@ def list_groups(db: Session, *, tenant_id: int, include_inactive: bool = False): .group_by(ClientGroup.id) .order_by(ClientGroup.group_name.asc()) ) + if viewer_partner_id is not None: + review_access = exists(select(ClientServiceSubscription.id).where( + ClientServiceSubscription.tenant_id == tenant_id, + ClientServiceSubscription.client_id == Client.id, + ClientServiceSubscription.review_partner_user_id == viewer_partner_id, + ClientServiceSubscription.is_active.is_(True), + )) + stmt = stmt.where(or_(Client.partner_id == viewer_partner_id, review_access)) if not include_inactive: stmt = stmt.where(ClientGroup.is_active.is_(True)) return [{"group": group, "client_count": int(count or 0)} for group, count in db.execute(stmt).all()] @@ -37,8 +46,17 @@ def get_group_by_code(db: Session, *, tenant_id: int, group_code: str): return db.execute(select(ClientGroup).where(ClientGroup.tenant_id == tenant_id, ClientGroup.group_code == code)).scalar_one_or_none() -def list_group_clients(db: Session, *, tenant_id: int, group_id: int): - return db.execute(select(Client).where(Client.tenant_id == tenant_id, Client.client_group_id == group_id, Client.is_archived.is_(False)).order_by(Client.is_group_head.desc(), Client.client_name.asc())).scalars().all() +def list_group_clients(db: Session, *, tenant_id: int, group_id: int, viewer_partner_id: int | None = None): + stmt = select(Client).where(Client.tenant_id == tenant_id, Client.client_group_id == group_id, Client.is_archived.is_(False)) + if viewer_partner_id is not None: + review_access = exists(select(ClientServiceSubscription.id).where( + ClientServiceSubscription.tenant_id == tenant_id, + ClientServiceSubscription.client_id == Client.id, + ClientServiceSubscription.review_partner_user_id == viewer_partner_id, + ClientServiceSubscription.is_active.is_(True), + )) + stmt = stmt.where(or_(Client.partner_id == viewer_partner_id, review_access)) + return db.execute(stmt.order_by(Client.is_group_head.desc(), Client.client_name.asc())).scalars().all() def create_group(db: Session, *, tenant_id: int, actor_user_id: int, payload: dict): diff --git a/app/modules/client_groups/ui.py b/app/modules/client_groups/ui.py index 751f507..c5ee55c 100644 --- a/app/modules/client_groups/ui.py +++ b/app/modules/client_groups/ui.py @@ -10,12 +10,20 @@ from app.core.templating import templates from app.modules.core.rbac.deps import get_user_permissions, get_user_roles from app.modules.core.rbac.permission_guard import require_permission from app.modules.client_groups.service import GROUP_TYPES, create_group, get_group, list_group_clients, list_groups, update_group -from app.modules.clients.access import build_scope +from app.modules.clients.access import build_scope, enforce_partner_scope, is_partner_role from app.modules.clients import repository from app.modules.consultants.service import list_consultants router = APIRouter(prefix="/client-groups", tags=["client-groups-ui"]) +def _roles(db, user): + return {str(name).strip().lower() for name in get_user_roles(db, user.id)} + +def _scope(request, user, db): + roles = _roles(db, user) + scope = build_scope(request, user, lambda code: code in set(get_user_permissions(db, user.id))) + return enforce_partner_scope(scope, user=user, role_names=roles), roles + def _ctx(request, user, db, **extra): data={"request":request,"current_user":user,"current_user_roles":get_user_roles(db,user.id),"current_user_permissions":get_user_permissions(db,user.id),"csrf_token":get_or_create_csrf_token(request)}; data.update(extra); return data @@ -32,8 +40,8 @@ def groups_page(request: Request): try: user=get_current_user(request,db=db) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.view'); scope=build_scope(request,user,lambda code: True) - return templates.TemplateResponse('modules/client_groups/templates/client_groups/list.html',_ctx(request,user,db,title='Client Groups',groups=list_groups(db,tenant_id=scope.tenant_id,include_inactive=True))) + require_permission(db,user,'clients.view'); scope, roles=_scope(request,user,db) + return templates.TemplateResponse('modules/client_groups/templates/client_groups/list.html',_ctx(request,user,db,title='Client Groups',groups=list_groups(db,tenant_id=scope.tenant_id,include_inactive=True,viewer_partner_id=user.id if is_partner_role(roles) else None))) finally: db.close() @router.get("/new") @@ -42,8 +50,8 @@ def group_new(request: Request): try: user=get_current_user(request,db=db) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.create'); scope=build_scope(request,user,lambda code: True) - return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Add Client Group',row=None,group_types=GROUP_TYPES,partners=repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[])) + require_permission(db,user,'clients.create'); scope, roles=_scope(request,user,db) + return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Add Client Group',row=None,group_types=GROUP_TYPES,partners=([repository.get_partner(db,user.id)] if is_partner_role(roles) else repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None)),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[])) finally: db.close() @router.post("") @@ -52,9 +60,12 @@ async def group_create(request: Request): try: user=get_current_user(request,db=db); form=await request.form(); validate_csrf(request,form.get('csrf_token')) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.create'); scope=build_scope(request,user,lambda code: True) - try: row=create_group(db,tenant_id=scope.tenant_id,actor_user_id=user.id,payload=_payload(form)); return RedirectResponse(f'/client-groups/{row.id}',303) - except Exception as exc: return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Add Client Group',row=_payload(form),group_types=GROUP_TYPES,partners=repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[str(exc)]),status_code=400) + require_permission(db,user,'clients.create'); scope, roles=_scope(request,user,db) + try: + payload=_payload(form) + if is_partner_role(roles): payload["assigned_partner_user_id"] = user.id + row=create_group(db,tenant_id=scope.tenant_id,actor_user_id=user.id,payload=payload); return RedirectResponse(f'/client-groups/{row.id}',303) + except Exception as exc: return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Add Client Group',row=_payload(form),group_types=GROUP_TYPES,partners=([repository.get_partner(db,user.id)] if is_partner_role(roles) else repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None)),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[str(exc)]),status_code=400) finally: db.close() @router.get("/{group_id}") @@ -63,9 +74,9 @@ def group_detail(request: Request, group_id:int): try: user=get_current_user(request,db=db) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.view'); scope=build_scope(request,user,lambda code: True); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) + require_permission(db,user,'clients.view'); scope, roles=_scope(request,user,db); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) if not row: return RedirectResponse('/client-groups',303) - return templates.TemplateResponse('modules/client_groups/templates/client_groups/detail.html',_ctx(request,user,db,title=row.group_name,row=row,clients=list_group_clients(db,tenant_id=scope.tenant_id,group_id=row.id))) + return templates.TemplateResponse('modules/client_groups/templates/client_groups/detail.html',_ctx(request,user,db,title=row.group_name,row=row,clients=list_group_clients(db,tenant_id=scope.tenant_id,group_id=row.id,viewer_partner_id=user.id if is_partner_role(roles) else None))) finally: db.close() @router.get("/{group_id}/edit") @@ -74,9 +85,9 @@ def group_edit(request: Request, group_id:int): try: user=get_current_user(request,db=db) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.edit'); scope=build_scope(request,user,lambda code: True); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) + require_permission(db,user,'clients.edit'); scope, roles=_scope(request,user,db); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) if not row: return RedirectResponse('/client-groups',303) - return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Edit Client Group',row=row,group_types=GROUP_TYPES,partners=repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[])) + return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Edit Client Group',row=row,group_types=GROUP_TYPES,partners=([repository.get_partner(db,user.id)] if is_partner_role(roles) else repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None)),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[])) finally: db.close() @router.post("/{group_id}/edit") @@ -85,8 +96,11 @@ async def group_update(request: Request, group_id:int): try: user=get_current_user(request,db=db); form=await request.form(); validate_csrf(request,form.get('csrf_token')) if not user: return RedirectResponse('/login',303) - require_permission(db,user,'clients.edit'); scope=build_scope(request,user,lambda code: True); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) + require_permission(db,user,'clients.edit'); scope, roles=_scope(request,user,db); row=get_group(db,tenant_id=scope.tenant_id,group_id=group_id) if not row: return RedirectResponse('/client-groups',303) - try: update_group(db,row=row,actor_user_id=user.id,payload=_payload(form)); return RedirectResponse(f'/client-groups/{row.id}',303) - except Exception as exc: return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Edit Client Group',row=row,group_types=GROUP_TYPES,partners=repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[str(exc)]),status_code=400) + try: + payload=_payload(form) + if is_partner_role(roles): payload["assigned_partner_user_id"] = user.id + update_group(db,row=row,actor_user_id=user.id,payload=payload); return RedirectResponse(f'/client-groups/{row.id}',303) + except Exception as exc: return templates.TemplateResponse('modules/client_groups/templates/client_groups/form.html',_ctx(request,user,db,title='Edit Client Group',row=row,group_types=GROUP_TYPES,partners=([repository.get_partner(db,user.id)] if is_partner_role(roles) else repository.list_partners_for_scope(db,tenant_id=scope.tenant_id,branch_id=None)),consultants=list_consultants(db,tenant_id=scope.tenant_id,include_inactive=False),errors=[str(exc)]),status_code=400) finally: db.close() diff --git a/app/modules/clients/__init__.py b/app/modules/clients/__init__.py index 2c30557..219f58a 100644 --- a/app/modules/clients/__init__.py +++ b/app/modules/clients/__init__.py @@ -1,29 +1,17 @@ -"""Clients module. +"""Clients module with lazy router exports. -Routers are exposed lazily so importing ``app.modules.clients.models`` does -not initialise the clients API, UI, importer, and related modules. This keeps -the existing ``api_router`` and ``ui_router`` public exports while preventing -circular imports with client groups. +Importing client models must not initialise the complete UI/import stack. """ - from __future__ import annotations - from typing import Any __all__ = ["api_router", "ui_router"] - def __getattr__(name: str) -> Any: if name == "api_router": from .api import router - return router - if name == "ui_router": from .ui import router - return router - - raise AttributeError( - f"module {__name__!r} has no attribute {name!r}" - ) \ No newline at end of file + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/app/modules/clients/access.py b/app/modules/clients/access.py index dbbb23c..049b2d7 100644 --- a/app/modules/clients/access.py +++ b/app/modules/clients/access.py @@ -47,6 +47,21 @@ def build_scope(request, user, permission_checker): ) + +def is_partner_role(role_names) -> bool: + names = {str(name).strip().lower() for name in (role_names or [])} + return "partner" in names and "firm admin" not in names and "system admin" not in names + + +def enforce_partner_scope(scope: ClientAccessScope, *, user, role_names) -> ClientAccessScope: + if is_partner_role(role_names): + scope.allow_all_clients = False + scope.own_only = True + scope.locked_partner_id = int(user.id) + scope.can_assign_partner = False + return scope + + def effective_partner_id(row: dict): return row.get("assoc_partner_user_id") or row.get("partner_id") @@ -71,6 +86,6 @@ def can_view_client_row(scope: ClientAccessScope, row: dict, *, user_id: int) -> return False if scope.own_only and scope.locked_partner_id and effective_partner_id(row) != scope.locked_partner_id: - return False + return bool(row.get("has_review_access")) return True diff --git a/app/modules/clients/api.py b/app/modules/clients/api.py index 1c80c8b..b4c9929 100644 --- a/app/modules/clients/api.py +++ b/app/modules/clients/api.py @@ -7,7 +7,7 @@ from sqlalchemy.orm import Session from app.core.db.deps import get_common_db from app.core.security.session_auth import require_login -from app.modules.clients.access import ClientAccessScope +from app.modules.clients.access import ClientAccessScope, enforce_partner_scope, is_partner_role from app.modules.clients.schemas import ClientAuditLogOut, ClientFilterOptions, ClientListResponse, ClientOut, ClientUpdate, ClientCreate from app.modules.clients.service import ( activate_client_service, @@ -23,6 +23,7 @@ from app.modules.clients.service import ( update_client_service, ) from app.modules.core.rbac.permission_guard import require_permission +from app.modules.core.rbac.deps import get_user_roles router = APIRouter(prefix="/api/v1/clients", tags=["clients-api"]) @@ -34,17 +35,19 @@ def _api_scope_from_user(db, user): except Exception: return False own_only = has("clients.view.own_only") or not has("clients.assign_partner") - return ClientAccessScope( + scope = ClientAccessScope( tenant_id=user.tenant_id, branch_id=user.branch_id, allow_cross_branch=has("clients.cross_branch"), allow_cross_tenant=has("clients.cross_tenant"), + allow_all_clients=has("clients.view.all") and not own_only, own_only=own_only, locked_partner_id=user.id if own_only else None, can_assign_partner=has("clients.assign_partner"), can_change_branch=has("clients.cross_branch"), can_change_tenant=has("clients.cross_tenant"), ) + return enforce_partner_scope(scope, user=user, role_names=get_user_roles(db, user.id)) @router.get("/filters", response_model=ClientFilterOptions) def api_client_filters(): @@ -73,7 +76,8 @@ def api_list_clients( tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, - partner_id=partner_id, + partner_id=None if scope.own_only else partner_id, + viewer_partner_id=user.id if scope.own_only else None, q=q, status=status, client_type=client_type, @@ -105,7 +109,8 @@ def api_export_clients( tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, - partner_id=partner_id, + partner_id=None if scope.own_only else partner_id, + viewer_partner_id=user.id if scope.own_only else None, q=q, status=status, client_type=client_type, @@ -122,9 +127,7 @@ def api_export_clients( def api_get_client(client_id: int, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.view") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) - if scope.own_only and row.partner_id != scope.locked_partner_id: - raise HTTPException(status_code=404, detail="Client not found.") + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None, allow_review_access=True) return row @router.post("", response_model=ClientOut, status_code=201) @@ -137,7 +140,7 @@ def api_create_client(data: ClientCreate, db: Session = Depends(get_common_db), def api_update_client(client_id: int, data: ClientUpdate, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.edit") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) if scope.own_only and row.partner_id != scope.locked_partner_id: raise HTTPException(status_code=404, detail="Client not found.") return update_client_service(db, row=row, data=data, actor_user_id=user.id, scope=scope) @@ -146,33 +149,33 @@ def api_update_client(client_id: int, data: ClientUpdate, db: Session = Depends( def api_deactivate_client(client_id: int, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.deactivate") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) return deactivate_client_service(db, row=row, actor_user_id=user.id) @router.post("/{client_id}/activate", response_model=ClientOut) def api_activate_client(client_id: int, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.activate") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) return activate_client_service(db, row=row, actor_user_id=user.id) @router.post("/{client_id}/archive", response_model=ClientOut) def api_archive_client(client_id: int, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.archive") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) return archive_client_service(db, row=row, actor_user_id=user.id) @router.post("/{client_id}/restore", response_model=ClientOut) def api_restore_client(client_id: int, db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.restore") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) return restore_client_service(db, row=row, actor_user_id=user.id) @router.get("/{client_id}/audit-logs", response_model=list[ClientAuditLogOut]) def api_client_audit_logs(client_id: int, limit: int = Query(50, ge=1, le=200), db: Session = Depends(get_common_db), user=Depends(require_login)): require_permission(db, user, "clients.audit_log.view") scope = _api_scope_from_user(db, user) - row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch) + row = get_client_or_404(db, client_id=client_id, tenant_id=scope.tenant_id, branch_id=scope.branch_id, allow_cross_branch=scope.allow_cross_branch, allow_all_clients=scope.allow_all_clients, viewer_partner_id=user.id if scope.own_only else None) return list_client_audit_logs(db, row=row, limit=limit) diff --git a/app/modules/clients/repository.py b/app/modules/clients/repository.py index 815821b..09c2aa2 100644 --- a/app/modules/clients/repository.py +++ b/app/modules/clients/repository.py @@ -2,7 +2,7 @@ from __future__ import annotations from math import ceil -from sqlalchemy import asc, case, desc, func, or_, select +from sqlalchemy import and_, asc, case, desc, exists, func, or_, select from sqlalchemy.orm import Session from app.core.security.passwords import hash_password @@ -13,6 +13,7 @@ from app.modules.client_groups.models import ClientGroup from app.modules.core.iam.models import User from app.modules.core.rbac.models import Role, UserRole from app.modules.core.tenancy.models import Branch, Tenant +from app.modules.services.models import ClientServiceSubscription def _safe_sort(sort_by: str, sort_order: str): @@ -28,6 +29,7 @@ def build_clients_query( allow_cross_branch: bool = False, allow_all_clients: bool = False, partner_id: int | None = None, + viewer_partner_id: int | None = None, q: str = "", status: str = "", client_type: str = "", @@ -35,6 +37,14 @@ def build_clients_query( include_archived: bool = False, ): assoc = ClientAssociation + review_access = exists( + select(ClientServiceSubscription.id).where( + ClientServiceSubscription.tenant_id == tenant_id, + ClientServiceSubscription.client_id == Client.id, + ClientServiceSubscription.review_partner_user_id == viewer_partner_id, + ClientServiceSubscription.is_active.is_(True), + ) + ) if viewer_partner_id is not None else None stmt = ( select( Client, @@ -48,6 +58,7 @@ def build_clients_query( assoc.consultant_id.label("assoc_consultant_id"), assoc.partner_user_id.label("assoc_partner_user_id"), assoc.created_source.label("assoc_created_source"), + (review_access if review_access is not None else False).label("has_review_access"), ) .outerjoin(assoc, assoc.client_id == Client.id) .outerjoin(ClientGroup, ClientGroup.id == Client.client_group_id) @@ -61,7 +72,11 @@ def build_clients_query( stmt = stmt.where(Client.is_archived.is_(False)) if branch_id and not allow_all_clients and not allow_cross_branch: stmt = stmt.where(Client.branch_id == branch_id) - if partner_id: + if viewer_partner_id is not None: + stmt = stmt.where( + or_(Client.partner_id == viewer_partner_id, assoc.partner_user_id == viewer_partner_id, review_access) + ) + elif partner_id: stmt = stmt.where((Client.partner_id == partner_id) | (assoc.partner_user_id == partner_id)) if status: stmt = stmt.where(Client.status == status) @@ -82,13 +97,13 @@ def build_clients_query( def list_clients( db: Session, *, tenant_id: int, branch_id: int | None = None, allow_cross_branch: bool = False, allow_all_clients: bool = False, - partner_id: int | None = None, q: str = "", status: str = "", client_type: str = "", + partner_id: int | None = None, viewer_partner_id: int | None = None, q: str = "", status: str = "", client_type: str = "", client_group_id: int | None = None, include_archived: bool = False, page: int = 1, per_page: int = 25, sort_by: str = "client_name", sort_order: str = "asc", ) -> dict: stmt = build_clients_query( tenant_id=tenant_id, branch_id=branch_id, allow_cross_branch=allow_cross_branch, - allow_all_clients=allow_all_clients, partner_id=partner_id, q=q, status=status, + allow_all_clients=allow_all_clients, partner_id=partner_id, viewer_partner_id=viewer_partner_id, q=q, status=status, client_type=client_type, client_group_id=client_group_id, include_archived=include_archived, ) total = db.execute(select(func.count()).select_from(stmt.subquery())).scalar_one() @@ -100,7 +115,7 @@ def list_clients( stmt.order_by(_safe_sort(sort_by, sort_order)).offset(offset).limit(per_page) ).all() rows=[] - for client, partner_name, branch_name, tenant_name, client_group_name, client_group_code, association_type, assoc_firm_tenant_id, assoc_consultant_id, assoc_partner_user_id, assoc_created_source in result: + for client, partner_name, branch_name, tenant_name, client_group_name, client_group_code, association_type, assoc_firm_tenant_id, assoc_consultant_id, assoc_partner_user_id, assoc_created_source, has_review_access in result: row={**client.__dict__}; row.pop("_sa_instance_state",None) row.update({ "partner_name":partner_name,"branch_name":branch_name,"tenant_name":tenant_name, @@ -108,17 +123,39 @@ def list_clients( "association_type":association_type,"assoc_firm_tenant_id":assoc_firm_tenant_id, "assoc_consultant_id":assoc_consultant_id,"assoc_partner_user_id":assoc_partner_user_id, "assoc_created_source":assoc_created_source,"effective_partner_id":assoc_partner_user_id or row.get("partner_id"), + "has_review_access": bool(has_review_access), }); rows.append(row) stats_stmt=select(func.count(Client.id),func.sum(case((Client.status=="active",1),else_=0)),func.sum(case((Client.status=="inactive",1),else_=0)),func.sum(case((Client.status=="archived",1),else_=0))) if not allow_all_clients: stats_stmt=stats_stmt.where(Client.tenant_id==tenant_id) if branch_id and not allow_cross_branch: stats_stmt=stats_stmt.where(Client.branch_id==branch_id) - if partner_id: stats_stmt=stats_stmt.where(Client.partner_id==partner_id) + if viewer_partner_id is not None: + review_stats = exists(select(ClientServiceSubscription.id).where( + ClientServiceSubscription.tenant_id == tenant_id, + ClientServiceSubscription.client_id == Client.id, + ClientServiceSubscription.review_partner_user_id == viewer_partner_id, + ClientServiceSubscription.is_active.is_(True), + )) + stats_stmt = stats_stmt.where(or_(Client.partner_id == viewer_partner_id, review_stats)) + elif partner_id: + stats_stmt=stats_stmt.where(Client.partner_id==partner_id) total_all,active,inactive,archived=db.execute(stats_stmt).one() return {"rows":rows,"meta":{"total":total,"page":page,"per_page":per_page,"pages":pages},"stats":{"total":int(total_all or 0),"active":int(active or 0),"inactive":int(inactive or 0),"archived":int(archived or 0)}} -def get_client_detail_payload(db: Session, client_id: int): + +def has_partner_review_access(db: Session, *, tenant_id: int, client_id: int, partner_user_id: int) -> bool: + return bool(db.execute( + select(ClientServiceSubscription.id).where( + ClientServiceSubscription.tenant_id == tenant_id, + ClientServiceSubscription.client_id == client_id, + ClientServiceSubscription.review_partner_user_id == partner_user_id, + ClientServiceSubscription.is_active.is_(True), + ).limit(1) + ).scalar_one_or_none()) + + +def get_client_detail_payload(db: Session, client_id: int, *, viewer_partner_id: int | None = None): assoc=ClientAssociation stmt=(select( Client, ClientGroup.group_name.label("client_group_name"), ClientGroup.group_code.label("client_group_code"), @@ -133,7 +170,10 @@ def get_client_detail_payload(db: Session, client_id: int): row.update({"client_group_name":client_group_name,"client_group_code":client_group_code,"association_type":association_type, "assoc_firm_tenant_id":assoc_firm_tenant_id,"assoc_consultant_id":assoc_consultant_id, "assoc_partner_user_id":assoc_partner_user_id,"assoc_created_source":assoc_created_source, - "effective_partner_id":assoc_partner_user_id or row.get("partner_id")}) + "effective_partner_id":assoc_partner_user_id or row.get("partner_id"), + "has_review_access": bool(viewer_partner_id is not None and has_partner_review_access( + db, tenant_id=int(row["tenant_id"]), client_id=int(row["id"]), partner_user_id=int(viewer_partner_id) + ))}) return row diff --git a/app/modules/clients/service.py b/app/modules/clients/service.py index 4029e0f..ec8d23d 100644 --- a/app/modules/clients/service.py +++ b/app/modules/clients/service.py @@ -30,16 +30,15 @@ from app.modules.core.iam.models import User from app.modules.core.rbac.models import Role, UserRole from app.modules.documents.models import PermanentClientDocument from app.modules.consultants.service import sync_primary_client_consultant_link - from app.modules.clients.models import Client - def _get_client_group(*args, **kwargs): - # Imported lazily to prevent clients ↔ client_groups circular imports. from app.modules.client_groups.service import get_group return get_group(*args, **kwargs) + + def _payload_from_schema(data): payload = data.model_dump(exclude_none=True) if hasattr(data, "model_dump") else data.dict(exclude_none=True) # primary_consultant_id belongs to ClientConsultantLink, not the clients table. @@ -367,6 +366,8 @@ def get_client_or_404( branch_id: int | None, allow_cross_branch: bool, allow_all_clients: bool = False, + viewer_partner_id: int | None = None, + allow_review_access: bool = False, ): row = repository.get_client_by_id(db, client_id) if not row: @@ -375,6 +376,11 @@ def get_client_or_404( raise HTTPException(status_code=404, detail="Client not found in current tenant.") if not allow_all_clients and not allow_cross_branch and branch_id and row.branch_id != branch_id: raise HTTPException(status_code=404, detail="Client not found in current branch.") + if viewer_partner_id is not None and int(row.partner_id or 0) != int(viewer_partner_id): + if not allow_review_access or not repository.has_partner_review_access( + db, tenant_id=tenant_id, client_id=client_id, partner_user_id=viewer_partner_id + ): + raise HTTPException(status_code=404, detail="Client not found.") return row diff --git a/app/modules/clients/templates/clients/list.html b/app/modules/clients/templates/clients/list.html index b868070..3d3b6dc 100644 --- a/app/modules/clients/templates/clients/list.html +++ b/app/modules/clients/templates/clients/list.html @@ -51,12 +51,8 @@ - {% if client_group_id %} - - {% endif %} - {% if partner_id %} - - {% endif %} + {% if client_group_id %}{% endif %} + {% if partner_id %}{% endif %} @@ -73,12 +69,8 @@ {% if meta.pages > 1 %}