N+1 Pattern in No Allocation Reminder

[mrrobot47]

Severity: High

Category: Database/Scheduled Tasks

Location

• File: next_pms/resource_management/tasks/no_allocation_reminder.py
• Lines: 63-113
• Function: send_reminder()

Description

The no-allocation reminder task iterates through managers and their direct reports, making database queries for each reportee's leaves. Additionally, it uses inefficient O(n) list scans for allocation lookups within a day-by-day loop.

Problematic Code

for employee, data in employees.items():
    if not data.get("childrens"):
        continue
    # ...
    for r_employee in data.get("childrens"):
        # ...
        employee_allocations = [
            allocation for allocation in allocations 
            if allocation.get("employee") == r_employee.get("name")
        ]  # O(n) scan per employee
        
        leaves = get_employee_leaves(  # DB query per employee
            employee=r_employee.get("name"),
            start_date=next_monday,
            end_date=next_sunday,
        )

        while _start_date <= _end_date:  # Day-by-day loop
            # Multiple O(n) scans inside loop
            if not any(
                allocation.get("allocation_start_date") <= _start_date <= allocation.get("allocation_end_date")
                for allocation in employee_allocations
            ):
                if not any(
                    leave.get("from_date") <= _start_date <= leave.get("to_date") 
                    for leave in leaves
                ):
                    emp_missing_allocations["dates"].append(...)
            _start_date = add_days(_start_date, 1)

Impact

• For a company with 50 managers, each with 5 direct reports:
  • 250 get_employee_leaves() queries
  • 250 * 5 (weekdays) = 1250 allocation list scans
  • 250 * 5 = 1250 leave list scans
• Total: 250+ queries + 2500+ O(n) list operations

Recommended Fix

def send_reminder():
    doc = get_doc("Timesheet Settings")
    date = getdate()
    weekday = date.weekday()
    designations = [d.designation for d in doc.designations]
    
    if (
        not doc.send_missing_allocation_reminder
        or get_weekday(date) != doc.remind_on
        or not designations
        or not doc.allocation_email_template
    ):
        return

    employees = generate_flat_tree(
        "Employee",
        nsm_field="reports_to",
        filters={"status": "Active"},
        fields=["name", "user_id", "employee_name", "designation"],
    )
    if not employees:
        return
    employees = employees.get("with_children", [])
    if not employees:
        return

    next_monday = add_days(date, (7 - weekday) % 7 + 0)
    next_sunday = add_days(date, (7 - weekday) % 7 + 6)
    
    # Get all employee names (managers + their children)
    all_employee_names = set(employees.keys())
    for emp, data in employees.items():
        for child in data.get("childrens", []):
            all_employee_names.add(child.get("name"))
    
    # Batch fetch allocations for all employees
    allocations = get_allocation_list_for_employee_for_given_range(
        columns=[
            "name", "employee", "allocation_start_date", "allocation_end_date",
            "hours_allocated_per_day", "project"
        ],
        value_key="employee",
        values=list(all_employee_names),
        start_date=next_monday,
        end_date=next_sunday,
    )
    
    # Build allocation map: employee -> list of (start_date, end_date) tuples
    allocation_map = {}
    for alloc in allocations:
        emp = alloc.get("employee")
        if emp not in allocation_map:
            allocation_map[emp] = []
        allocation_map[emp].append((alloc.allocation_start_date, alloc.allocation_end_date))
    
    # Batch fetch leaves for all employees
    leaves = frappe.db.sql("""
        SELECT employee, from_date, to_date
        FROM `tabLeave Application`
        WHERE employee IN %(employees)s
        AND status IN ('Approved', 'Open')
        AND from_date <= %(end_date)s
        AND to_date >= %(start_date)s
    """, {
        "employees": list(all_employee_names),
        "start_date": next_monday,
        "end_date": next_sunday
    }, as_dict=True)
    
    # Build leave map
    leave_map = {}
    for leave in leaves:
        emp = leave.employee
        if emp not in leave_map:
            leave_map[emp] = []
        leave_map[emp].append((leave.from_date, leave.to_date))
    
    # Pre-generate all dates for the week
    week_dates = []
    current = next_monday
    while current <= next_sunday:
        if current.weekday() not in SKIP_WEEKDAYS:
            week_dates.append(current)
        current = add_days(current, 1)
    
    args = []
    
    for employee, data in employees.items():
        if not data.get("childrens"):
            continue
        
        arg = {
            "employee": employee,
            "employee_name": data.get("employee_name"),
            "user_id": data.get("user_id"),
            "missing_allocations": [],
        }
        
        for r_employee in data.get("childrens"):
            if r_employee.get("designation") not in designations:
                continue
            
            emp_name = r_employee.get("name")
            emp_allocations = allocation_map.get(emp_name, [])
            emp_leaves = leave_map.get(emp_name, [])
            
            missing_dates = []
            for check_date in week_dates:
                # Check if date is covered by any allocation
                has_allocation = any(
                    start <= check_date <= end 
                    for start, end in emp_allocations
                )
                if has_allocation:
                    continue
                
                # Check if date is covered by any leave
                has_leave = any(
                    start <= check_date <= end 
                    for start, end in emp_leaves
                )
                if has_leave:
                    continue
                
                missing_dates.append(check_date.strftime("%Y-%m-%d"))
            
            if missing_dates:
                arg["missing_allocations"].append({
                    "employee": emp_name,
                    "employee_name": r_employee.get("employee_name"),
                    "designation": r_employee.get("designation"),
                    "dates": missing_dates,
                })
        
        if arg["missing_allocations"]:
            args.append(arg)
    
    if not args:
        return
    
    # Send emails (template fetched once)
    template = get_doc("Email Template", doc.allocation_email_template)
    for arg in args:
        message = render_template(
            template.response if not template.use_html else template.response_html, 
            arg
        )
        subject = render_template(template.subject, arg)
        sendmail(recipients=arg["user_id"], subject=subject, message=message)

Estimated Performance Gain

• Before: 250+ queries + 2500+ list scans
• After: 3 batch queries (allocations, leaves, template)
• Improvement: ~99% reduction in database queries
• Time: 20-40s -> 1-2s

Testing Recommendations

1. Verify managers receive correct missing allocation data
2. Test with employees who have partial week allocations
3. Ensure weekend days are properly skipped
4. Validate leave handling during allocation check