Fix N+1 query pattern in availability check for members

[Copilot]

This PR fixes the N+1 query pattern in the check_availability() function by implementing request-level caching for member availability data.

Problem

The check_availability() function is called for every date when calculating available appointment slots. Inside this function, it loads the User Appointment Availability document for each mandatory member in a loop, resulting in redundant database queries.

With 5 mandatory members and a 10-day availability window, a single time slot request results in:

• 10 days × 5 members = 50 get_doc() calls just for this one function

Solution Implemented

Implemented Option 2 from the proposed solutions: request-level caching using frappe.local.

Changes Made

1. Added get_member_availability_cached() helper function

   • Caches member availability data in frappe.local.appointment_cache
   • Fetches each member's availability only once per request
   • Stores both days and time_slots for potential reuse in other functions

2. Updated check_availability() function

   • Modified to use the cached data instead of calling frappe.get_doc() for each member on every date check
   • Maintains the same functionality with significantly reduced database queries

Performance Impact

• Before: 10 days × 5 members = 50 get_doc() calls
• After: 5 get_doc() calls (one per unique member, cached for the request)
• Improvement: 90% reduction in database queries for this function

This aligns with the estimated 50-70% overall performance gain mentioned in the issue.

Testing

• ✅ All linting checks pass (ruff)
• ✅ Code review feedback addressed
• ✅ Security scan completed with no vulnerabilities (CodeQL)
• ✅ Minimal changes following the principle of surgical modifications

The implementation is minimal and focused, affecting only the caching mechanism without altering the core logic or functionality of appointment slot calculations.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>N+1 Query Pattern in Availability Check for Members</issue_title>
<issue_description>

Metadata

• File(s): frappe_appointment/frappe_appointment/doctype/appointment_group/appointment_group.py:292-298
• Category: Database / API
• Severity: High
• Effort to Fix: Medium
• Estimated Performance Gain: 50-70%

Problem Description

The check_availability() function is called for every date when calculating available appointment slots. Inside this function, it loads the User Appointment Availability document for each mandatory member in a loop.

This function is called:

• During the daily availability verification task (for each day in the window)
• During API calls to get available time slots
• When booking appointments to validate slots

With 5 mandatory members and a 10-day availability window, a single time slot request results in:

• 10 days × 5 members = 50 get_doc() calls just for this one function

Code Location

check_availability() - lines 292-298:

def check_availability(date_validation_obj: object, weekday: str, appointment_group: object) -> object:
    # ...
    mandatory_members = [member for member in appointment_group.members if member.is_mandatory]
    available_days = set(ALL_DAYS)

    for member in mandatory_members:
        availability = frappe.get_doc("User Appointment Availability", member.user)  # N+1
        user_available_days = [day.day for day in availability.appointment_time_slot]
        available_days = available_days.intersection(set(user_available_days))

    res["available_days"] = available_days
    # ...

Root Cause

1. Each member's availability document is loaded individually
2. This function is called for EACH date being checked
3. The availability data doesn't change during a request, but is re-fetched every time
4. No caching mechanism in place

Proposed Solution

Option 1: Batch fetch at the start of slot calculation

def _get_time_slots_for_given_date(appointment_group: object, datetime: datetime):
    date = datetime.date()
    weekday = get_weekday(datetime)
    
    # Batch fetch all member availabilities upfront
    mandatory_members = [m for m in appointment_group.members if m.is_mandatory]
    member_user_ids = [m.user for m in mandatory_members]
    
    # Get all appointment time slots for all members in one query
    all_time_slots = frappe.get_all(
        "Appointment Time Slot",
        filters={"parent": ["in", member_user_ids]},
        fields=["parent", "day", "start_time", "end_time"]
    )
    
    # Group by member
    member_time_slots = {}
    for slot in all_time_slots:
        if slot.parent not in member_time_slots:
            member_time_slots[slot.parent] = []
        member_time_slots[slot.parent].append(slot)
    
    # Calculate available days without loading full docs
    available_days = set(ALL_DAYS)
    for member_id in member_user_ids:
        user_days = {slot.day for slot in member_time_slots.get(member_id, [])}
        available_days = available_days.intersection(user_days)
    
    # Continue with rest of logic using member_time_slots...

Option 2: Add request-level caching

def get_member_availability_cached(member_user):
    """Get member availability with request-level caching."""
    cache_key = f"member_availability_{member_user}"
    
    if not hasattr(frappe.local, 'appointment_cache'):
        frappe.local.appointment_cache = {}
    
    if cache_key not in frappe.local.appointment_cache:
        availability = frappe.get_doc("User Appointment Availability", member_user)
        frappe.local.appointment_cache[cache_key] = {
            'days': [day.day for day in availability.appointment_time_slot],
            'time_slots': availability.appointment_time_slot
        }
    
    return frappe.local.appointment_cache[cache_key]


def check_availability(date_validation_obj: object, weekday: str, appointment_group: object) -> object:
    # ...
    mandatory_members = [member for member in appointment_group.members if member.is_mandatory]
    available_days = set(ALL_DAYS)

    for member in mandatory_members:
        cached = get_member_availability_cached(member.user)  # Cached lookup
        available_days = available_days.intersection(set(cached['days']))

    res["available_days"] = available_days
    # ...

Implementation Steps

1. Create a helper function to batch fetch member availabilities
2. Add request-level caching for availability data using frappe.local
3. Pass the cached data through the call chain instead of re-fetching
4. Update check_availability() to use cached data
5. Test with appointment groups having multiple members
6. Verify time slot calculations remain accurate

Additional Notes

• This same N+1 pattern appears in is_member_on_leave_or_is_holiday() (see Issue 4)
• The _get_time_slots_for_day() function calls thi...

• Fixes N+1 Query Pattern in Availability Check for Members #254

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[mrrobot47]

@copilot continue

[Copilot]

@copilot continue

I've implemented the fix for the N+1 query pattern by adding request-level caching for member availability data. The changes are minimal and focused:

1. Added get_member_availability_cached() helper function that caches member availability in frappe.local.appointment_cache
2. Updated check_availability() to use cached data instead of repeatedly calling frappe.get_doc()

With 5 mandatory members and a 10-day availability window, this reduces database queries from 50 to just 5 (90% reduction).

Latest commit: 09964ad