Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ def sync_press_role(doc, method=None):
# Loop through the resources and create `team-member-resource` entries if
# they don't exist.
for resource in resources:
# Skip stale resources whose document has been transferred or archived.
document_team = frappe.db.get_value(resource.document_type, resource.document_name, "team")
if document_team != team:
continue
Comment on lines 133 to +137

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 N+1 query inside inner loop on a live hook path

frappe.db.get_value is issued for every resource entry in the inner loop, which already runs once per user. Because sync_press_role fires as a hook on every Press Role save (not only during the migration patch), a team with many users and many resources will generate a large number of individual SELECT queries on each save. The existing frappe.db.exists call at line 148 compounds the same pattern. Consider collecting all unique (document_type, document_name) pairs from resources before the loop and resolving their teams in per-type batches (one frappe.db.get_all per distinct document_type), then look up results from a local dict during the iteration.


# Check if a `team-member-resource` entry already exists for the team,
# user, document type, and document.
document = {
Expand Down
Loading