N+1 Query Pattern in copy_crm_notes_to_opportunity

[mrrobot47]

Severity: High

Category: Database/Background Jobs

File: next_crm/api/crm_note.py

Lines: 201-302

Description

The copy_crm_notes_to_opportunity function is called when converting a Lead to an Opportunity. It exhibits multiple N+1 patterns:

1. Uses fields="*" to fetch notes
2. Calls get_doc() for each child note
3. Calls frappe.get_all() for attachments per note
4. Creates/saves documents in a loop
5. Has a final frappe.db.commit() at the end

Code Evidence

# Line 202-211 - fields="*" for parent notes
notes = frappe.get_all(
    "CRM Note",
    fields="*",  # Fetches all columns
    filters={...},
)

for note in notes:
    new_parent_note = frappe.new_doc("CRM Note")
    # ... set fields ...
    new_parent_note.insert()  # N+1 insert
    
    # Line 224-228 - N+1 attachment query per note
    attachments = frappe.get_all(
        "NCRM Attachments",
        filters={"parent": note.name, "parenttype": "CRM Note"},
        fields=["filename"],
    )
    
    for row in attachments:
        new_file_name = duplicate_file(...)  # N+1 file operations
        
    # Line 246-252 - set_value per note
    frappe.db.set_value("CRM Note", new_parent_note.name, {"owner": note.owner})
    
    # Line 254-257 - Another fields="*" for child notes
    child_notes = frappe.get_all(
        "CRM Note",
        filters={"custom_parent_note": note.name},
        fields="*",
    )
    
    for child_note in child_notes:
        # Same pattern repeats for child notes...
        new_child_note.insert()  # N+1 insert
        # N+1 attachment query
        # N+1 file operations
        frappe.db.set_value(...)  # N+1 set_value

frappe.db.commit()  # Single commit at end (good)

Impact

For a Lead with 10 parent notes, 20 child notes, and 30 attachments:

• 50+ INSERT queries (one per note)
• 30+ SELECT queries for attachments
• 30+ file operations for duplicating
• 30+ UPDATE queries for set_value

Total: ~150+ database operations for a single Lead conversion

Proposed Solution

1. Batch fetch all notes and attachments upfront
2. Use bulk insert where possible
3. Specify only required fields

def copy_crm_notes_to_opportunity(lead, opportunity):
    # Batch fetch all notes with required fields only
    all_notes = frappe.get_all(
        "CRM Note",
        filters={"parent": lead, "parenttype": "Lead"},
        fields=["name", "custom_title", "note", "owner", "added_by", "added_on", "custom_parent_note"],
        order_by="creation asc",
    )
    
    parent_notes = [n for n in all_notes if not n.custom_parent_note]
    child_notes_by_parent = {}
    for n in all_notes:
        if n.custom_parent_note:
            child_notes_by_parent.setdefault(n.custom_parent_note, []).append(n)
    
    # Batch fetch all attachments for all notes at once
    note_names = [n.name for n in all_notes]
    all_attachments = frappe.get_all(
        "NCRM Attachments",
        filters={"parent": ["in", note_names], "parenttype": "CRM Note"},
        fields=["parent", "filename"],
    )
    attachments_by_note = {}
    for att in all_attachments:
        attachments_by_note.setdefault(att.parent, []).append(att.filename)
    
    # Now process with pre-fetched data
    for note in parent_notes:
        new_parent_note = create_note_copy(note, opportunity, attachments_by_note.get(note.name, []))
        
        for child_note in child_notes_by_parent.get(note.name, []):
            create_note_copy(child_note, opportunity, attachments_by_note.get(child_note.name, []), 
                           parent=new_parent_note.name)
    
    frappe.db.commit()

Estimated Performance Gain

• 80-90% reduction in database queries
• Lead conversion time reduced from seconds to milliseconds

Verification

Monitor query count during Lead to Opportunity conversion before and after fix.