Feat/enhanced role based access control for my audits report

audit_management/audit_management/doctype/my_audits/my_audits.py

@@ -802,9 +802,7 @@ def has_permission(doc, ptype, user=None):
     # 4. Access for Audit Members (Non-Managers)
    # Audit Member: only if NOT draft OR owner
     if "Audit Member" in roles:
-        if getattr(doc, "status", None) == "Draft":
-            return doc.owner == user
-        return True
+        return doc.owner == user
 
     # 5. Access for Others (Owner or Current Assignee)
     if doc.owner == user:
@@ -827,66 +825,129 @@ def get_permission_query_conditions(user=None):
 
     roles = frappe.get_roles(user)
 
+    # =========================================================
+    # ADMIN BYPASS
+    # =========================================================
     if "Administrator" in roles or "System Manager" in roles:
         return ""
 
+    # =========================================================
+    # DIVISION ACCESS
+    # =========================================================
     allowed_divisions = get_user_allowed_divisions(user)
-    divisions_sql = ", ".join(f"{frappe.db.escape(d)}" for d in allowed_divisions) if allowed_divisions else "'None'"
 
-    # NEW: Sol ID Condition
+    divisions_sql = ", ".join(
+        [frappe.db.escape(d) for d in allowed_divisions]
+    ) if allowed_divisions else "'None'"
+
+    # =========================================================
+    # AUDIT MANAGER
+    # Full division access
+    # =========================================================
+    if "Audit Manager" in roles:
+        return f"""
+            `tabMy Audits`.emp_division IN ({divisions_sql})
+        """
+
+    # =========================================================
+    # AUDIT MEMBER
+    # Only created records
+    # =========================================================
+    if "Audit Member" in roles:
+        return f"""
+            `tabMy Audits`.owner = '{user}'
+        """
+
+    # =========================================================
+    # SOL ID ACCESS
+    # =========================================================
     allowed_sol_ids = get_user_allowed_sol_ids(user)
-    sol_id_condition = "1=0"
+
+    sol_condition = "1=0"
+
     if allowed_sol_ids:
-        sol_ids_str = ", ".join([frappe.db.escape(str(s)) for s in allowed_sol_ids])
-        sol_id_condition = f"""
-            `tabMy Audits`.emp_branch IN (
-                SELECT name FROM `tabAudit Level` 
-                WHERE sahayog_branch IN ({sol_ids_str})
+        sol_ids_sql = ", ".join(
+            [frappe.db.escape(str(s)) for s in allowed_sol_ids]
+        )
+
+        sol_condition = f"""
+            (
+                `tabMy Audits`.status != 'Draft'
+                AND
+                `tabMy Audits`.emp_branch IN (
+                    SELECT name
+                    FROM `tabAudit Level`
+                    WHERE sahayog_branch IN ({sol_ids_sql})
+                )
             )
         """
-
-    is_audit_manager = "Audit Manager" in roles
-    is_audit_team = is_audit_manager or "Audit Member" in roles
-
-    # ✅ FIX: correct child table name
+    # =========================================================
+    # STAGE ACCESS
+    # =========================================================
     pending_condition = f"""
         EXISTS (
-            SELECT name FROM `tabAudit Items`
+            SELECT name
+            FROM `tabAudit Items`
             WHERE parent = `tabMy Audits`.name
             AND status = 'Pending'
-            AND (user_id = '{user}' OR email = '{user}')
+            AND (
+                user_id = '{user}'
+                OR email = '{user}'
+            )
         )
     """
+
     responded_condition = f"""
-    EXISTS (
-        SELECT name FROM `tabAudit Items`
-        WHERE parent = `tabMy Audits`.name
-        AND status = 'Responded'
-        AND (user_id = '{user}' OR email = '{user}')
-    )
+        EXISTS (
+            SELECT name
+            FROM `tabAudit Items`
+            WHERE parent = `tabMy Audits`.name
+            AND status = 'Responded'
+            AND (
+                user_id = '{user}'
+                OR email = '{user}'
+            )
+        )
     """
-    if is_audit_manager:
-      return f"`tabMy Audits`.emp_division IN ({divisions_sql})"
-
-    if "Audit Member" in roles and not is_audit_manager:
-      return f"`tabMy Audits`.owner = '{user}'"
 
-    # ✅ FINAL CONTROL
+    # =========================================================
+    # FINAL CONDITIONS
+    # =========================================================
     return f"""
         (
-            (`tabMy Audits`.status = 'Draft' AND `tabMy Audits`.owner = '{user}')
-            OR
-            (`tabMy Audits`.owner = '{user}' AND `tabMy Audits`.emp_division IN ({divisions_sql}))
+
+            -- Draft only owner
+            (
+                `tabMy Audits`.status = 'Draft'
+                AND `tabMy Audits`.owner = '{user}'
+            )
+
             OR
-            ({sol_id_condition})
+
+            -- SOL ID based access
+            (
+                {sol_condition}
+            )
+
             OR
+
+            -- Pending stage access
             (
-                `tabMy Audits`.status != 'Draft' AND (
-                    ({pending_condition})
+                `tabMy Audits`.status != 'Draft'
+                AND (
+                    {pending_condition}
                     OR
-                    ({responded_condition})
+                    {responded_condition}
                 )
             )
+
+            OR
+
+            -- Owner access
+            (
+                `tabMy Audits`.owner = '{user}'
+            )
+
         )
     """
 

audit_management/audit_management/report/my_audits_report/my_audits_report.py

@@ -64,15 +64,10 @@ def get_data(filters):
         # Sees everything, no perm filter needed
         pass
     elif is_audit_member:
-        # Audit Member: Sees only their own created records (as requested) 
-        # OR records in their allowed divisions
-        allowed_divisions = get_user_allowed_divisions(user)
-        perm_conds = [f"owner = {frappe.db.escape(user)}"]
-        if allowed_divisions:
-            div_list = ", ".join([frappe.db.escape(d) for d in allowed_divisions])
-            perm_conds.append(f"emp_division IN ({div_list})")
-
-        conditions.append(f"({' OR '.join(perm_conds)})")
+    # Audit Member: only own created records
+        conditions.append(
+            f"owner = {frappe.db.escape(user)}"
+        )
     else:
         # Other users: Sol ID based access (from Report Preference) 
         # OR records where they are participants
@@ -82,18 +77,29 @@ def get_data(filters):
         if allowed_sol_ids:
             sol_list = ", ".join([frappe.db.escape(str(s)) for s in allowed_sol_ids])
             perm_conds.append(f"""
-                emp_branch IN (
-                    SELECT name FROM `tabAudit Level` 
-                    WHERE sahayog_branch IN ({sol_list})
+                (
+                    status != 'Draft'
+                    AND
+                    emp_branch IN (
+                        SELECT name FROM `tabAudit Level`
+                        WHERE sahayog_branch IN ({sol_list})
+                    )
                 )
             """)
 
         # Also include where they are assigned (Audit Items)
         perm_conds.append(f"""
-            EXISTS (
-                SELECT name FROM `tabAudit Items`
-                WHERE parent = `tabMy Audits`.name
-                AND (user_id = {frappe.db.escape(user)} OR email = {frappe.db.escape(user)})
+            (
+                status != 'Draft'
+                AND EXISTS (
+                    SELECT name
+                    FROM `tabAudit Items`
+                    WHERE parent = `tabMy Audits`.name
+                    AND (
+                        user_id = {frappe.db.escape(user)}
+                        OR email = {frappe.db.escape(user)}
+                    )
+                )
             )
         """)
 

audit_management/audit_management/utils.py

@@ -29,7 +29,7 @@ def update_audit_aging(doc):
     end_date = getdate(doc.modified) if doc.status == "Closed" else getdate(nowdate())
 
     doc.aging = get_working_days(start_date, end_date)
-
+    
 def get_user_allowed_divisions(user=None):
     """
     Fetch all divisions user can access.
@@ -57,7 +57,7 @@ def get_user_allowed_divisions(user=None):
     settings = frappe.get_single("Audit Management Settings")
 
     if not getattr(settings, "division_permissions", None):
-        return list(allowed_divisions)
+        return [d for d in allowed_divisions if d]
 
     # Add mapped divisions
     for row in settings.division_permissions:
@@ -67,4 +67,4 @@ def get_user_allowed_divisions(user=None):
         ):
             allowed_divisions.add(row.allowed_division)
 
-    return list(allowed_divisions)
+    return [d for d in allowed_divisions if d]