N+1 Query Pattern in delete_attachment API

[mrrobot47]

Severity: Medium

Category: Database/API

File: next_crm/api/activities.py

Lines: 681-701

Description

The delete_attachment API loads each CRM Note using get_doc() in a loop when searching for notes that contain the attachment to delete.

Code Evidence

if doctype and docname:
    notes = frappe.get_all(
        "CRM Note",
        filters={"parenttype": doctype, "parent": docname},
        fields=["name"],
    )

    for note in notes:
        note_doc = frappe.get_doc("CRM Note", note.name)  # N+1 query
        original_count = len(note_doc.custom_note_attachments)

        updated_attachments = [
            row
            for row in note_doc.custom_note_attachments
            if row.filename != filename
        ]

        if len(updated_attachments) != original_count:
            note_doc.set("custom_note_attachments", updated_attachments)
            note_doc.save()  # N+1 save
            deleted = True

Impact

For a document with 20 notes:

• 20 get_doc() calls to check each note
• Potentially 20 save() calls if multiple notes have the attachment

Proposed Solution

Query the attachment child table directly to find which notes contain the file:

if doctype and docname:
    # Find notes that have this attachment directly
    notes_with_attachment = frappe.get_all(
        "NCRM Attachments",
        filters={"filename": filename},
        fields=["parent"],
        pluck="parent",
    )
    
    if notes_with_attachment:
        # Filter to notes belonging to this document
        notes_to_update = frappe.get_all(
            "CRM Note",
            filters={
                "name": ["in", notes_with_attachment],
                "parenttype": doctype,
                "parent": docname,
            },
            fields=["name"],
            pluck="name",
        )
        
        for note_name in notes_to_update:
            # Delete the attachment row directly
            frappe.db.delete(
                "NCRM Attachments",
                {"parent": note_name, "filename": filename}
            )
            deleted = True

Estimated Performance Gain

• 90% reduction in queries when document has many notes
• Only loads notes that actually have the attachment