N+1 in Project Threshold Reminder

[mrrobot47]

Severity: Critical

Category: Database/Scheduled Tasks

Location

• File: next_pms/project_currency/tasks/reminde_project_threshold.py
• Lines: 78-106, 29-55
• Functions: filter_project_list(), send_reminder_mail_for_project()

Description

The weekly project threshold reminder task has a severe N+1 pattern. It first fetches a list of projects, then for each project calls frappe.get_doc() to load the full document. For each project that needs a reminder, it makes additional queries for DocShare and Has Role.

Problematic Code

filter_project_list() - N+1 on Project:

def filter_project_list(project_list: list):
    need_to_send_reminder_project_list = []
    for project_name in project_list:
        project = frappe.get_doc("Project", project_name)  # N+1: Full doc load per project

        if project.custom_billing_type == "Retainer":
            custom_project_budget_hours = project.custom_project_budget_hours
            # ... calculate threshold
        elif project.custom_billing_type == "Time and Material":
            # ... calculate threshold
        
        if project.custom_reminder_threshold_percentage <= project_threshold:
            need_to_send_reminder_project_list.append(project)
    
    return need_to_send_reminder_project_list

send_reminder_mail_for_project() - Additional N+1:

def send_reminder_mail_for_project(project: str):
    user_list = frappe.get_all(
        "DocShare",
        fields=["name", "user"],
        filters=dict(share_doctype=project.doctype, share_name=project.name),
    )  # Query per project

    all_pms = [
        d.parent
        for d in frappe.get_all(
            "Has Role",
            filters={"role": "Projects Manager", "parenttype": "User", "parent": ["in", user_list]},
            fields=["parent"],
        )
    ]  # Query per project

    reminder_template = frappe.get_doc("Email Template", project.custom_email_template)  # Query per project

Impact

• Database Load: For 100 open projects with reminders enabled:
  • 100 frappe.get_doc("Project") calls (with child tables = 300+ queries)
  • 100 DocShare queries
  • 100 Has Role queries
  • 100 Email Template queries
  • Total: 600+ queries for what should be 5-6 batch queries
• Weekly Execution: Runs every week, causing periodic performance degradation

Recommended Fix

def send_reminder_mail():
    try:
        # Get all required project fields upfront, including child table data
        projects = frappe.get_all(
            "Project",
            filters={
                "custom_send_reminder_when_approaching_project_threshold_limit": 1,
                "status": "Open",
            },
            fields=[
                "name", "custom_billing_type", "custom_reminder_threshold_percentage",
                "custom_email_template", "custom_default_hourly_billing_rate"
            ],
        )
        
        if not projects:
            return
        
        project_names = [p.name for p in projects]
        
        # Batch fetch project budget hours for all projects
        budget_hours = frappe.get_all(
            "Project Budget Hours",  # Actual child table name
            filters={"parent": ["in", project_names]},
            fields=["parent", "hours_purchased", "consumed_hours", "idx"],
            order_by="parent, idx"
        )
        
        # Group budget hours by project
        budget_map = {}
        for bh in budget_hours:
            if bh.parent not in budget_map:
                budget_map[bh.parent] = []
            budget_map[bh.parent].append(bh)
        
        # Filter projects that need reminders
        projects_needing_reminder = []
        for project in projects:
            project_budget = budget_map.get(project.name, [])
            threshold = calculate_threshold(project, project_budget)
            
            if threshold is not None and project.custom_reminder_threshold_percentage <= threshold:
                project.calculated_threshold = threshold
                projects_needing_reminder.append(project)
        
        if not projects_needing_reminder:
            return
        
        reminder_project_names = [p.name for p in projects_needing_reminder]
        
        # Batch fetch DocShare for all projects needing reminder
        doc_shares = frappe.get_all(
            "DocShare",
            filters={"share_doctype": "Project", "share_name": ["in", reminder_project_names]},
            fields=["share_name", "user"]
        )
        
        # Group by project
        share_map = {}
        for ds in doc_shares:
            if ds.share_name not in share_map:
                share_map[ds.share_name] = []
            share_map[ds.share_name].append(ds.user)
        
        # Get all users who are Project Managers
        all_shared_users = list(set(ds.user for ds in doc_shares))
        if all_shared_users:
            pm_roles = frappe.get_all(
                "Has Role",
                filters={
                    "role": "Projects Manager",
                    "parenttype": "User",
                    "parent": ["in", all_shared_users]
                },
                pluck="parent"
            )
            pm_set = set(pm_roles)
        else:
            pm_set = set()
        
        # Batch fetch email templates
        template_names = list(set(p.custom_email_template for p in projects_needing_reminder if p.custom_email_template))
        templates = {}
        if template_names:
            template_docs = frappe.get_all(
                "Email Template",
                filters={"name": ["in", template_names]},
                fields=["name", "use_html", "response_html", "response", "subject"]
            )
            templates = {t.name: t for t in template_docs}
        
        # Now send reminders without additional queries
        for project in projects_needing_reminder:
            if not project.custom_email_template:
                continue
            
            template = templates.get(project.custom_email_template)
            if not template:
                continue
            
            shared_users = share_map.get(project.name, [])
            recipients = [u for u in shared_users if u in pm_set]
            
            if not recipients:
                continue
            
            email_message = template.response_html if template.use_html else template.response
            message = frappe.render_template(email_message, {"project": project})
            subject = frappe.render_template(template.subject, {"project": project})
            
            frappe.sendmail(recipients=recipients, subject=subject, message=message)
            
    except Exception:
        generate_the_error_log("send_reminder_project_threshold_mail_failed")


def calculate_threshold(project, budget_hours):
    """Calculate threshold percentage from project and budget data."""
    if project.custom_billing_type == "Retainer":
        if not budget_hours:
            return None
        last_budget = budget_hours[-1]
        if last_budget.hours_purchased:
            return (last_budget.consumed_hours * 100) / last_budget.hours_purchased
    elif project.custom_billing_type == "Time and Material":
        # Note: Original code has a bug - uses undefined variable
        # This needs proper implementation based on actual requirements
        pass
    return None

Estimated Performance Gain

• Before: 600+ queries for 100 projects
• After: 6-8 batch queries total
• Improvement: ~99% reduction in database queries
• Time: 30s -> 1-2s

Additional Notes

• The original code has a bug in the "Time and Material" branch - it references custom_project_budget_hours which is undefined at that point
• Email templates are loaded once per unique template instead of once per project