N+1 in get_compact_view_data API

[mrrobot47]

Severity: High

Category: Database/API

Location

• File: next_pms/timesheet/api/team.py
• Lines: 106-164
• Function: get_compact_view_data()

Description

The get_compact_view_data() API endpoint, which powers the team timesheet compact view, loops through employees and makes multiple database queries per employee. This is called frequently as users navigate the timesheet interface.

Problematic Code

for employee in employees:
    working_hours = get_employee_working_hours(employee.name)  # DB query (cached)
    daily_working_hours = get_employee_daily_working_norm(employee.name)  # DB query (cached)
    local_data = {**employee, **working_hours}
    employee_timesheets = timesheet_map.get(employee.name, [])

    status = get_timesheet_state(  # Potentially expensive
        employee=employee.name,
        start_date=dates[0].get("start_date"),
        end_date=dates[-1].get("end_date"),
    )
    local_data["status"] = status
    local_data["data"] = []

    leaves = get_employee_leaves(  # DB query per employee
        start_date=add_days(dates[0].get("start_date"), -max_week * 7),
        end_date=add_days(dates[-1].get("end_date"), max_week * 7),
        employee=employee.name,
    )
    holidays = get_holidays(employee.name, dates[0].get("start_date"), dates[-1].get("end_date"))  # DB query per employee

    for date_info in dates:
        for date in date_info.get("dates"):
            # O(n) scans for leaves and holidays per date
            for leave in leaves:
                if leave["from_date"] <= date <= leave["to_date"]:
                    # ...
            for holiday in holidays:
                if date == holiday.holiday_date:
                    # ...

Impact

• For a page showing 10 employees over 2 weeks:
  • 10 * get_employee_working_hours() = 10 queries (may be cached)
  • 10 * get_employee_daily_working_norm() = 10 queries (may be cached)
  • 10 * get_timesheet_state() = 10 queries
  • 10 * get_employee_leaves() = 10 queries
  • 10 * get_holidays() = 10 queries
• Total: 50 queries per page load
• API response time: 500ms-2s when it should be <100ms

Recommended Fix

@whitelist()
@error_logger
def get_compact_view_data(
    date: str,
    max_week: int = 2,
    employee_name=None,
    employee_ids: list[str] | str | None = None,
    department=None,
    project=None,
    user_group=None,
    page_length=10,
    start=0,
    status_filter=None,
    status=None,
    reports_to: str | None = None,
    by_pass_access_check=False,
):
    if not by_pass_access_check:
        only_for(["Timesheet Manager", "Timesheet User", "Projects Manager"], message=True)

    dates = []
    data = {}
    if status_filter and isinstance(status_filter, str):
        status_filter = json.loads(status_filter)

    for i in range(max_week):
        week = get_week_dates(date=date)
        dates.append(week)
        date = add_days(getdate(week["start_date"]), -1)

    dates.reverse()
    res = {"dates": dates}
    
    start_date = dates[0].get("start_date")
    end_date = dates[-1].get("end_date")
    extended_start = add_days(start_date, -max_week * 7)
    extended_end = add_days(end_date, max_week * 7)

    employees, total_count = filter_employee_by_timesheet_status(
        employee_name=employee_name,
        department=department,
        project=project,
        user_group=user_group,
        page_length=page_length,
        start=start,
        reports_to=reports_to,
        status=status,
        timesheet_status=status_filter,
        employee_ids=employee_ids,
        start_date=start_date,
        end_date=end_date,
    )
    
    if not employees:
        return {"dates": dates, "data": {}, "total_count": 0, "has_more": False}
    
    employee_names = [employee.name for employee in employees]

    # Batch fetch employee working hours
    emp_work_data = frappe.get_all(
        "Employee",
        filters={"name": ["in", employee_names]},
        fields=["name", "custom_working_hours", "custom_work_schedule"]
    )
    emp_work_map = {e.name: e for e in emp_work_data}
    
    # Default working hours from settings
    default_hours = frappe.db.get_single_value("HR Settings", "standard_working_hours") or 8

    # Batch fetch all timesheets
    timesheet_data = get_all(
        "Timesheet",
        filters={
            "employee": ["in", employee_names],
            "start_date": [">=", start_date],
            "end_date": ["<=", end_date],
            "docstatus": ["!=", 2],
        },
        fields=["employee", "start_date", "end_date", "total_hours", "note", "custom_approval_status"],
    )
    
    timesheet_map = {}
    for ts in timesheet_data:
        if ts.employee not in timesheet_map:
            timesheet_map[ts.employee] = []
        timesheet_map[ts.employee].append(ts)

    # Batch fetch all leaves for all employees
    all_leaves = frappe.db.sql("""
        SELECT employee, from_date, to_date, half_day, half_day_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": employee_names,
        "start_date": extended_start,
        "end_date": extended_end
    }, as_dict=True)
    
    leave_map = {}
    for leave in all_leaves:
        if leave.employee not in leave_map:
            leave_map[leave.employee] = []
        leave_map[leave.employee].append(leave)

    # Batch fetch holiday lists for all employees
    emp_holiday_lists = frappe.get_all(
        "Employee",
        filters={"name": ["in", employee_names]},
        fields=["name", "holiday_list"]
    )
    holiday_list_names = list(set(e.holiday_list for e in emp_holiday_lists if e.holiday_list))
    
    # Fetch holidays for all holiday lists in date range
    holidays_by_list = {}
    if holiday_list_names:
        holidays = frappe.get_all(
            "Holiday",
            filters={
                "parent": ["in", holiday_list_names],
                "holiday_date": ["between", [start_date, end_date]]
            },
            fields=["parent", "holiday_date", "weekly_off"]
        )
        for h in holidays:
            if h.parent not in holidays_by_list:
                holidays_by_list[h.parent] = []
            holidays_by_list[h.parent].append(h)
    
    emp_holiday_map = {e.name: e.holiday_list for e in emp_holiday_lists}

    # Batch get timesheet states
    all_states = batch_get_timesheet_states(employee_names, start_date, end_date)

    # Process employees without additional queries
    for employee in employees:
        emp_work = emp_work_map.get(employee.name, {})
        working_hour = emp_work.get("custom_working_hours") or default_hours
        working_frequency = emp_work.get("custom_work_schedule") or "Per Day"
        
        daily_working_hours = working_hour / 5 if working_frequency != "Per Day" else working_hour
        
        local_data = {
            **employee,
            "working_hour": working_hour,
            "working_frequency": working_frequency,
        }
        
        employee_timesheets = timesheet_map.get(employee.name, [])
        local_data["status"] = all_states.get(employee.name, {})
        local_data["data"] = []

        leaves = leave_map.get(employee.name, [])
        holiday_list = emp_holiday_map.get(employee.name)
        holidays = holidays_by_list.get(holiday_list, [])
        
        # Convert to sets for O(1) lookup
        holiday_date_map = {h.holiday_date: h for h in holidays}

        for date_info in dates:
            for check_date in date_info.get("dates"):
                hour = 0
                on_leave = False

                # Check leaves
                for leave in leaves:
                    if leave["from_date"] <= check_date <= leave["to_date"]:
                        if leave.get("half_day") and leave.get("half_day_date") == check_date:
                            hour += daily_working_hours / 2
                        else:
                            hour += daily_working_hours
                        on_leave = True

                # Check holidays
                if check_date in holiday_date_map:
                    holiday = holiday_date_map[check_date]
                    hour = daily_working_hours if not holiday.weekly_off else 0
                    on_leave = False
                
                # Sum timesheet hours
                total_hours = 0
                notes = ""
                for ts in employee_timesheets:
                    if ts.start_date == check_date and ts.end_date == check_date:
                        total_hours += ts.get("total_hours")
                        notes += ts.get("note", "")
                hour += total_hours

                local_data["data"].append({
                    "date": check_date,
                    "hour": hour,
                    "is_leave": on_leave,
                    "note": notes.replace("<br>", "\n"),
                })

        data[employee.name] = local_data

    res["data"] = data
    res["total_count"] = total_count
    res["has_more"] = int(start) + int(page_length) < total_count

    return res


def batch_get_timesheet_states(employee_names, start_date, end_date):
    """Batch fetch timesheet states for all employees."""
    # Implementation depends on get_timesheet_state logic
    # Return dict of {employee_name: state_dict}
    states = {}
    # Batch implementation here
    return states

Estimated Performance Gain

• Before: 50 queries for 10 employees
• After: 5-7 batch queries total
• Improvement: ~90% reduction in queries
• Response time: 500ms-2s -> 50-100ms

Testing Recommendations

1. Verify UI displays same data before/after
2. Test with various page sizes
3. Ensure leave and holiday display is correct
4. Measure API response time improvement