From 9c70b4f8add4c00762f6d61152f23c8d7888ce5f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 12 Jan 2026 15:35:06 +0000 Subject: [PATCH 1/5] Initial plan From 9843509f85a94a0108a8444ea028801641e935a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:10:21 +0000 Subject: [PATCH 2/5] Refactor copy_crm_notes_to_opportunity to eliminate N+1 query pattern Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 187 ++++++++++++++++++++------------------- 1 file changed, 96 insertions(+), 91 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index d2e6842e01..b9b66bfa67 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -199,107 +199,112 @@ def delete_note(note_name): def copy_crm_notes_to_opportunity(lead, opportunity): - notes = frappe.get_all( + # Batch fetch all notes with required fields only + all_notes = frappe.get_all( "CRM Note", - fields="*", - filters={ - "parent": lead, - "parenttype": "Lead", - "custom_parent_note": ["in", ["", None]], - }, + filters={"parent": lead, "parenttype": "Lead"}, + fields=["name", "custom_title", "note", "owner", "added_by", "added_on", "custom_parent_note"], order_by="creation asc", ) - - for note in notes: - new_parent_note = frappe.new_doc("CRM Note") - new_parent_note.custom_title = note.custom_title or "" - new_parent_note.note = note.note or "" - new_parent_note.parenttype = "Opportunity" - new_parent_note.parent = opportunity - new_parent_note.parentfield = "notes" - new_parent_note.added_by = note.added_by - new_parent_note.added_on = note.added_on or now() - - new_parent_note.insert() - attachments = frappe.get_all( - "NCRM Attachments", - filters={"parent": note.name, "parenttype": "CRM Note"}, - fields=["filename"], + + if not all_notes: + return + + # Separate parent notes from child notes + 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"], + ) + + # Group attachments by parent note + attachments_by_note = {} + for att in all_attachments: + attachments_by_note.setdefault(att.parent, []).append(att.filename) + + # Process parent notes with pre-fetched data + for note in parent_notes: + new_parent_note = _create_note_copy( + note, + opportunity, + attachments_by_note.get(note.name, []) ) - for row in attachments: - new_file_name = duplicate_file( - row.filename, - new_attached_to_doctype="Opportunity", - new_attached_to_name=opportunity, + + # Process child notes for this parent + 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_note_name=new_parent_note.name ) - if new_file_name: - new_parent_note.append( - "custom_note_attachments", - { - "filename": new_file_name, - }, - ) - - if attachments: - new_parent_note.save() - - frappe.db.set_value( - "CRM Note", - new_parent_note.name, - { - "owner": note.owner, - }, - ) - - child_notes = frappe.get_all( - "CRM Note", - filters={"custom_parent_note": note.name}, - fields="*", - ) + + frappe.db.commit() - for child_note in child_notes: - new_child_note = frappe.new_doc("CRM Note") - new_child_note.custom_title = child_note.custom_title or "" - new_child_note.note = child_note.note or "" - new_child_note.parenttype = "Opportunity" - new_child_note.parent = opportunity - new_child_note.parentfield = "notes" - new_child_note.added_by = child_note.added_by - new_child_note.added_on = child_note.added_on or now() - new_child_note.custom_parent_note = new_parent_note.name - - new_child_note.insert() - child_attachments = frappe.get_all( - "NCRM Attachments", - filters={"parent": child_note.name, "parenttype": "CRM Note"}, - fields=["filename"], - ) - for row in child_attachments: - new_file_name = duplicate_file( - row.filename, - new_attached_to_doctype="Opportunity", - new_attached_to_name=opportunity, - ) - if new_file_name: - new_child_note.append( - "custom_note_attachments", - { - "filename": new_file_name, - }, - ) - - if child_attachments: - new_child_note.save() - - frappe.db.set_value( - "CRM Note", - new_child_note.name, +def _create_note_copy(note, opportunity, attachments, parent_note_name=None): + """ + Helper function to create a copy of a note with its attachments. + + Args: + note: The original note object with fields + opportunity: The opportunity name to attach the note to + attachments: List of attachment filenames for this note + parent_note_name: Optional parent note name for child notes + + Returns: + The newly created note document + """ + new_note = frappe.new_doc("CRM Note") + new_note.custom_title = note.custom_title or "" + new_note.note = note.note or "" + new_note.parenttype = "Opportunity" + new_note.parent = opportunity + new_note.parentfield = "notes" + new_note.added_by = note.added_by + new_note.added_on = note.added_on or now() + + if parent_note_name: + new_note.custom_parent_note = parent_note_name + + new_note.insert() + + # Process attachments for this note + for filename in attachments: + new_file_name = duplicate_file( + filename, + new_attached_to_doctype="Opportunity", + new_attached_to_name=opportunity, + ) + if new_file_name: + new_note.append( + "custom_note_attachments", { - "owner": child_note.owner, + "filename": new_file_name, }, ) - frappe.db.commit() + + if attachments: + new_note.save() + + # Set the owner to match the original note + frappe.db.set_value( + "CRM Note", + new_note.name, + { + "owner": note.owner, + }, + ) + + return new_note def duplicate_file( From 107679e48ade751bdfeb127c3a40bd4025439fb0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:11:44 +0000 Subject: [PATCH 3/5] Address code review feedback: use defaultdict and add early return comment Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index b9b66bfa67..a43094b3a7 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -1,3 +1,5 @@ +from collections import defaultdict + import frappe from frappe import _ from frappe.desk.doctype.notification_log.notification_log import ( @@ -207,15 +209,16 @@ def copy_crm_notes_to_opportunity(lead, opportunity): order_by="creation asc", ) + # Early return if no notes to copy if not all_notes: return # Separate parent notes from child notes parent_notes = [n for n in all_notes if not n.custom_parent_note] - child_notes_by_parent = {} + child_notes_by_parent = defaultdict(list) for n in all_notes: if n.custom_parent_note: - child_notes_by_parent.setdefault(n.custom_parent_note, []).append(n) + child_notes_by_parent[n.custom_parent_note].append(n) # Batch fetch all attachments for all notes at once note_names = [n.name for n in all_notes] @@ -226,9 +229,9 @@ def copy_crm_notes_to_opportunity(lead, opportunity): ) # Group attachments by parent note - attachments_by_note = {} + attachments_by_note = defaultdict(list) for att in all_attachments: - attachments_by_note.setdefault(att.parent, []).append(att.filename) + attachments_by_note[att.parent].append(att.filename) # Process parent notes with pre-fetched data for note in parent_notes: From 765a532ac171b7eb9c518e7f3d1e4662228fcc40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:12:45 +0000 Subject: [PATCH 4/5] Minor optimizations: guard clause for attachments and store child_notes in variable Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index a43094b3a7..439829cc76 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -222,11 +222,13 @@ def copy_crm_notes_to_opportunity(lead, opportunity): # 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"], - ) + all_attachments = [] + if note_names: + all_attachments = frappe.get_all( + "NCRM Attachments", + filters={"parent": ["in", note_names], "parenttype": "CRM Note"}, + fields=["parent", "filename"], + ) # Group attachments by parent note attachments_by_note = defaultdict(list) @@ -242,7 +244,8 @@ def copy_crm_notes_to_opportunity(lead, opportunity): ) # Process child notes for this parent - for child_note in child_notes_by_parent.get(note.name, []): + child_notes = child_notes_by_parent[note.name] + for child_note in child_notes: _create_note_copy( child_note, opportunity, From 2e395c846bf5f637dd0da98997b532d0fd4a9563 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 10:13:40 +0000 Subject: [PATCH 5/5] Remove redundant guard clause for note_names Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index 439829cc76..4e60885f3b 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -222,13 +222,11 @@ def copy_crm_notes_to_opportunity(lead, opportunity): # Batch fetch all attachments for all notes at once note_names = [n.name for n in all_notes] - all_attachments = [] - if note_names: - all_attachments = frappe.get_all( - "NCRM Attachments", - filters={"parent": ["in", note_names], "parenttype": "CRM Note"}, - fields=["parent", "filename"], - ) + all_attachments = frappe.get_all( + "NCRM Attachments", + filters={"parent": ["in", note_names], "parenttype": "CRM Note"}, + fields=["parent", "filename"], + ) # Group attachments by parent note attachments_by_note = defaultdict(list)