How should permission_query_conditions be combined when multiple Frappe apps define them for the same DocType?

[pacman-24]

🏷️ Discussion Type

Bug

Body

I'm working on a Frappe setup with multiple custom apps. I noticed an issue when more than one app defines permission_query_conditions for the same DocType.

For example, suppose App A and App B both define permission conditions for Sales Invoice.

Instead of behaving like:

App A condition OR App B condition

the resulting behavior seems to effectively require both conditions:

App A condition AND App B condition

This can unintentionally restrict the records a user can access.

What is the recommended way to handle this when multiple apps need to contribute to the permissions of the same DocType?

Should the permission_query_conditions from different apps be explicitly merged into a single function? If so, what is the recommended upgrade-safe approach for combining the conditions from multiple hooks.py / permissions.py implementations without one app overriding another?

Guidelines

• I have read and understood this category's guidelines before making this post.

[github-actions[bot]]

💬 Your Product Feedback Has Been Submitted 🎉

Thank you for taking the time to share your insights with us! Your feedback is invaluable as we build a better GitHub experience for all our users.

Here's what you can expect moving forward ⏩

• Your input will be carefully reviewed and cataloged by members of our product teams. 
  • Due to the high volume of submissions, we may not always be able to provide individual responses.
  • Rest assured, your feedback will help chart our course for product improvements.
• Other users may engage with your post, sharing their own perspectives or experiences. 
• GitHub staff may reach out for further clarification or insight. 
  • We may 'Answer' your discussion if there is a current solution, workaround, or roadmap/changelog post related to the feedback.

Where to look to see what's shipping 👀

• Read the Changelog for real-time updates on the latest GitHub features, enhancements, and calls for feedback.
• Explore our Product Roadmap, which details upcoming major releases and initiatives.

What you can do in the meantime 💻

• Upvote and comment on other user feedback Discussions that resonate with you.
• Add more information at any point! Useful details include: use cases, relevant labels, desired outcomes, and any accompanying screenshots.

As a member of the GitHub community, your participation is essential. While we can't promise that every suggestion will be implemented, we want to emphasize that your feedback is instrumental in guiding our decisions and priorities.

Thank you once again for your contribution to making GitHub even better! We're grateful for your ongoing support and collaboration in shaping the future of our platform. ⭐

[tanimomor]

Recommended Approach

When multiple Frappe apps define permission_query_conditions for the same DocType, the conditions are combined with AND, not OR.

For example, if App A and App B both define permission conditions for Sales Invoice:

# App A
def sales_invoice(user):
    return "`tabSales Invoice`.company = 'Company A'"

# App B
def sales_invoice(user):
    return "`tabSales Invoice`.owner = 'user@example.com'"

The effective condition is essentially:

WHERE
    `tabSales Invoice`.company = 'Company A'
    AND
    `tabSales Invoice`.owner = 'user@example.com'

This can unintentionally make the permission too restrictive.

If the intended business logic is OR, the conditions need to be explicitly combined into a single permission function:

def sales_invoice(user):
    conditions = [
        "`tabSales Invoice`.company = 'Company A'",
        "`tabSales Invoice`.owner = 'user@example.com'",
    ]

    return " OR ".join(f"({condition})" for condition in conditions)

This produces:

WHERE
    (
        `tabSales Invoice`.company = 'Company A'
        OR
        `tabSales Invoice`.owner = 'user@example.com'
    )

For multiple custom apps, avoid relying on hook ordering or allowing one app to overwrite another. If several apps need to contribute permission logic for the same DocType, use a deliberate composition strategy and keep the individual conditions separate from the final function that combines them.

The important point is that permission_query_conditions is a query restriction. Separate restrictions narrow the result set, so they should not be expected to automatically provide OR-based access control.

For an upgrade-safe implementation, keep the customization in your own app(s), avoid modifying Frappe/ERPNext core, and avoid depending on the order in which apps are installed or their hooks are loaded.

[pacman-24]

They got union issue though. should i put both in 1 app?

[tanimomor]

You can put permission states or criteria or mapping in each app and call only from 1 app and use it as source of truth