Optimize delete_attachment to eliminate N+1 query pattern

[Copilot]

The delete_attachment API loaded every CRM Note document individually when searching for attachment references, causing N+1 queries. For a document with 20 notes, this resulted in up to 41 database queries.

Changes

Query optimization

• Query NCRM Attachments child table directly to find notes containing the target file
• Filter results to notes belonging to the specified document
• Bulk delete matching attachment rows in single operation

Performance impact

• 20 notes scenario: 41 queries → 3 queries (~93% reduction)
• Only processes notes that actually contain the attachment

# Before: Load all notes, get_doc each one, filter and save
notes = frappe.get_all("CRM Note", filters={"parenttype": doctype, "parent": docname})
for note in notes:
    note_doc = frappe.get_doc("CRM Note", note.name)  # N+1
    # ... filter and save

# After: Query child table, filter, bulk delete
notes_with_attachment = frappe.get_all("NCRM Attachments", filters={"filename": filename}, pluck="parent")
notes_to_update = frappe.get_all("CRM Note", filters={"name": ["in", notes_with_attachment], ...}, pluck="name")
frappe.db.delete("NCRM Attachments", {"parent": ["in", notes_to_update], "filename": filename})

Original prompt

---

This section details on the original issue you should resolve

<issue_title>N+1 Query Pattern in delete_attachment API</issue_title>
<issue_description>

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</issue_description>

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

• Fixes N+1 Query Pattern in delete_attachment API #335

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.