Optimize delete_note to eliminate N+1 queries on child notes and attachments

[Copilot]

The delete_note API endpoint executed N queries for N child notes (loading each individually, deleting notifications one-by-one) and M queries for M attachments. For a parent note with 5 children and 10 attachments, this resulted in ~30 database queries.

Changes

Batch fetch attachments (lines 181-187)

• Single get_all query fetches all attachments for parent + children vs. N get_doc calls

Bulk delete notifications (line 190)

• Single delete with ["in", note_names] filter vs. N individual deletes

File cleanup preserved (lines 199-204)

• Kept loop with delete_doc to trigger Frappe's file cleanup hooks
• Alternative bulk SQL delete would risk orphaned files on disk

Performance

• Before: ~30 queries (5 get_doc, 6 notification deletes, 6 note deletes, 10 file deletes)
• After: ~17 queries (1 attachment fetch, 1 bulk notification delete, 6 note deletes, 10 file deletes)
• Reduction: ~50%

# Before: N+1 pattern
for child_note in child_notes:
    child_note_doc = frappe.get_doc("CRM Note", child_note)  # N queries
    child_filenames = [row.filename for row in child_note_doc.custom_note_attachments]
    frappe.db.delete("CRM Notification", {"notification_type_doc": child_note})  # N queries

# After: Single batched query
all_attachments = frappe.get_all(
    "NCRM Attachments",
    filters={"parent": ["in", note_names_to_delete], "parenttype": "CRM Note"},
    pluck="filename",
)
frappe.db.delete("CRM Notification", {"notification_type_doc": ["in", note_names_to_delete]})

Original prompt

---

This section details on the original issue you should resolve

<issue_title>N+1 Query Pattern in delete_note with Child Notes</issue_title>
<issue_description>

Severity: Medium

Category: Database/API

File: next_crm/api/crm_note.py

Lines: 162-198

Description

The delete_note API endpoint loads each child note using get_doc() in a loop, then deletes files one by one. This creates N+1 query patterns for notes with many child notes and attachments.

Code Evidence

@frappe.whitelist()
def delete_note(note_name):
    note = frappe.get_doc("CRM Note", note_name)
    filenames_to_delete = [row.filename for row in note.custom_note_attachments]

    parent_note = note.custom_parent_note
    if not parent_note:
        child_notes = frappe.get_all(
            "CRM Note",
            filters={"custom_parent_note": note_name},
            fields=["name"],
            pluck="name",
        )
        for child_note in child_notes:
            child_note_doc = frappe.get_doc("CRM Note", child_note)  # N+1 query
            child_filenames = [
                row.filename for row in child_note_doc.custom_note_attachments
            ]
            filenames_to_delete.extend(child_filenames)
            frappe.db.delete("CRM Notification", {"notification_type_doc": child_note})
            frappe.delete_doc("CRM Note", child_note)  # N+1 delete

    frappe.db.delete("CRM Notification", {"notification_type_doc": note_name})
    note.delete()
    
    for filename in filenames_to_delete:  # N+1 file deletes
        try:
            frappe.delete_doc("File", filename)
        except frappe.DoesNotExistError:
            pass

Impact

For a parent note with 5 child notes and 10 total attachments:

• 5 get_doc() calls for child notes
• 5 delete_doc() calls for child notes
• 10 delete_doc() calls for files
• 6+ notification delete queries

Total: ~30 queries for a single note deletion

Proposed Solution

1. Batch fetch child notes with attachments
2. Use bulk delete for notifications and files

@frappe.whitelist()
def delete_note(note_name):
    # Get parent note
    note = frappe.get_doc("CRM Note", note_name)
    note_names_to_delete = [note_name]
    
    # Get child notes if this is a parent
    if not note.custom_parent_note:
        child_notes = frappe.get_all(
            "CRM Note",
            filters={"custom_parent_note": note_name},
            fields=["name"],
            pluck="name",
        )
        note_names_to_delete.extend(child_notes)
    
    # Batch fetch all attachments for all notes
    all_attachments = frappe.get_all(
        "NCRM Attachments",
        filters={"parent": ["in", note_names_to_delete], "parenttype": "CRM Note"},
        fields=["filename"],
        pluck="filename",
    )
    
    # Bulk delete notifications
    frappe.db.delete("CRM Notification", {"notification_type_doc": ["in", note_names_to_delete]})
    
    # Bulk delete notes using SQL for efficiency
    for note_name_to_del in note_names_to_delete:
        frappe.delete_doc("CRM Note", note_name_to_del, ignore_permissions=True)
    
    # Bulk delete files
    if all_attachments:
        frappe.db.delete("File", {"name": ["in", all_attachments]})
    
    return True

Estimated Performance Gain

• 70-80% reduction in queries for notes with children
• Faster response time for delete operations

Verification

Test delete_note with a parent note that has 5+ child notes and 10+ attachments.</issue_description>

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

• Fixes N+1 Query Pattern in delete_note with Child Notes #334

---

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