Fix N+1 query pattern in comment visibility filter

[Copilot]

The filter_comments_by_visibility() function executes a database query per comment to check User Group Member records for "Visible to mentioned" visibility. A document with 50 comments (20 mentioned) triggers 21 queries: 1 for comments + 20 individual membership lookups.

Changes

• Batch query optimization: Collect all "Visible to mentioned" comment names upfront, fetch User Group Member records in a single query using ["in", comment_ids] filter
• O(1) set lookup: Replace per-comment queries with in-memory set membership check
• Early return: Skip processing entirely for Administrator user

Implementation

# Collect comment IDs needing membership check
mentioned_visibility_comments = [c.name for c in comments if c.custom_visibility == "Visible to mentioned"]

# Single batch query instead of N queries
if mentioned_visibility_comments:
    members = frappe.db.get_all(
        "User Group Member",
        filters={
            "user": user,
            "parent": ["in", mentioned_visibility_comments],  # Batch query
            "parenttype": "Comment",
        },
        fields=["parent"],
    )
    user_mentioned_in = {m.parent for m in members}

# O(1) lookup instead of query
if comment.owner == user or comment.name in user_mentioned_in:
    filtered_comments.append(comment)

Impact

Query reduction scales with comment count:

• 10 comments (5 mentioned): 6 → 2 queries (67% reduction)
• 50 comments (20 mentioned): 21 → 2 queries (90% reduction)
• 100 comments (40 mentioned): 41 → 2 queries (95% reduction)

Affects both add_comments_in_timeline() and get_all_replies() code paths.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>N+1 Query Pattern in Comment Visibility Filter</issue_title>
<issue_description>

Metadata

• File(s): frappe_comment_xt/helpers/comment.py:76-86
• Category: Database / API
• Severity: High
• Effort to Fix: Low
• Estimated Performance Gain: 70-90%

Problem Description

The filter_comments_by_visibility() function is called for every document view to filter comments based on visibility settings. For comments with "Visible to mentioned" visibility, it executes a database query per comment to check if the current user is in the mentioned users group.

A document with 50 comments, where 20 have "Visible to mentioned" visibility, results in:

• 1 initial query to get all comments
• 20 individual queries to check User Group Member for each "Visible to mentioned" comment
• Total: 21 queries per document view

Code Location

filter_comments_by_visibility() - lines 62-93:

def filter_comments_by_visibility(comments, user):
    filtered_comments = []

    if user != "Administrator":
        for comment in comments:
            if comment.custom_visibility == "Visible to only you":
                if comment.owner == user:
                    filtered_comments.append(comment)

            elif comment.custom_visibility == "Visible to mentioned":
                member = frappe.db.get_all(  # N+1 query per comment!
                    "User Group Member",
                    filters={
                        "user": user,
                        "parent": comment.name,
                        "parenttype": "Comment",
                    },
                )

                if comment.owner == user or (len(member) > 0):
                    filtered_comments.append(comment)

            else:
                filtered_comments.append(comment)
    else:
        filtered_comments = comments
    return filtered_comments

Root Cause

1. Query is executed inside the loop for each "Visible to mentioned" comment
2. No batching of the User Group Member lookups
3. This function is called on every document timeline view

Proposed Solution

Batch fetch all User Group Member records for the current user upfront:

def filter_comments_by_visibility(comments, user):
    """
    Filter comments based on the visibility settings
    Only show comments that the user is allowed to see
    """
    if user == "Administrator":
        return comments
    
    filtered_comments = []
    
    # Collect comment names that need mentioned-user check
    mentioned_visibility_comments = [
        c.name for c in comments 
        if c.custom_visibility == "Visible to mentioned"
    ]
    
    # Batch fetch all User Group Member records for this user
    user_mentioned_in = set()
    if mentioned_visibility_comments:
        members = frappe.db.get_all(
            "User Group Member",
            filters={
                "user": user,
                "parent": ["in", mentioned_visibility_comments],
                "parenttype": "Comment",
            },
            fields=["parent"]
        )
        user_mentioned_in = {m.parent for m in members}
    
    # Now filter without additional queries
    for comment in comments:
        if comment.custom_visibility == "Visible to only you":
            if comment.owner == user:
                filtered_comments.append(comment)
                
        elif comment.custom_visibility == "Visible to mentioned":
            # O(1) lookup instead of query
            if comment.owner == user or comment.name in user_mentioned_in:
                filtered_comments.append(comment)
                
        else:
            filtered_comments.append(comment)
    
    return filtered_comments

Implementation Steps

1. Collect all comment names with "Visible to mentioned" visibility
2. Execute a single batch query for User Group Member records
3. Build a set of comment names where user is mentioned
4. Use set lookup (O(1)) instead of per-comment query
5. Test with documents having many comments with mixed visibility settings

Additional Notes

• This function is also used in get_all_replies(), so the fix benefits multiple code paths
• Consider caching the user's mentioned-in set at request level if multiple calls occur
• The fields=["parent"] in the batch query returns only what's needed for the lookup</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes N+1 Query Pattern in Comment Visibility Filter #46

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.