Skip to content
This repository was archived by the owner on Jun 17, 2026. It is now read-only.
This repository was archived by the owner on Jun 17, 2026. It is now read-only.

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

Description

@mrrobot47

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.

Metadata

Metadata

Labels

mediumMedium severityperformancePerformance optimization

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions