From dd04b50fda597f3ec18bcaae2103fcb339b69ddf Mon Sep 17 00:00:00 2001 From: jtricerolph Date: Thu, 23 Jul 2026 14:11:14 +0000 Subject: [PATCH] Fix two root causes of timesheet figure overcount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Weekly fetch step (not per-day): timesheets/on/{date} returns the whole weekly period for any date in that week, so fetching 30 days counted each weekly timesheet ~7x. Now fetches one date per 7-day step from 'to' backwards (PBI pattern), deduplicates by timesheet.id, and filters shifts by sh.date to the requested window. 2. Filter by user.report_department_id not shift.department_id: shared departments (HR, Management, etc.) have a single location assignment (No 4) but staff from all hotels clock into them. Filtering by the employee's HOME department (report_department_id) correctly isolates No 4 staff. Mirrors the PBI model join: timesheet → user → report_department_id → department.location_id → hotel. Both fixes applied to syncActuals and compareEndpoints diagnostic. Target: B.cost = WF "Timesheet exc. leave inc. allowances" (~£51k June). Co-Authored-By: Claude Sonnet 4.6 --- backend/src/lib/workforce.js | 207 +++++++++++++++++++++-------------- 1 file changed, 124 insertions(+), 83 deletions(-) diff --git a/backend/src/lib/workforce.js b/backend/src/lib/workforce.js index cb0bc05..f519a6c 100644 --- a/backend/src/lib/workforce.js +++ b/backend/src/lib/workforce.js @@ -69,95 +69,114 @@ async function getEnabledDeptIds() { } } -async function getUserNameMap() { - const all = await wfFetchPaged('/api/v2/users') - return Object.fromEntries( - all.map(u => [String(u.id), u.name || `${u.legal_first_name || ''} ${u.legal_last_name || ''}`.trim() || `User ${u.id}`]) - ) -} - -// Build list of YYYY-MM-DD strings between from and to (inclusive) -function dateRange(from, to) { +// Build fetch dates covering [from, to]: step backwards from 'to' in 7-day increments, +// going one extra week before 'from' so weekly timesheets at the boundary are captured. +// Mirrors the PBI pattern (EndOfPeriod, EndOfPeriod-7, ...) — fetching one date per weekly +// timesheet period prevents counting the same record ~7x (one fetch per week, not per day). +function weeklyFetchDates(from, to) { const dates = [] - const cur = new Date(from + 'T00:00:00') - const end = new Date(to + 'T00:00:00') - while (cur <= end) { dates.push(cur.toISOString().slice(0, 10)); cur.setDate(cur.getDate() + 1) } + const limit = new Date(from + 'T00:00:00') + limit.setDate(limit.getDate() - 7) + let d = new Date(to + 'T00:00:00') + while (d >= limit) { + dates.push(d.toISOString().slice(0, 10)) + d.setDate(d.getDate() - 7) + } return dates } // Fetch timesheets for a single date — no location param (mirrors PBI pattern). -// Each record is { user_id, shifts: [{ department_id, date, cost, leave_request_id, ... }] } +// Returns the weekly timesheet record(s) containing this date. +// Each record: { id, user_id, shifts: [{ department_id, date, cost, leave_request_id, ... }] } async function fetchTimesheetsForDate(dateStr) { try { return await wfFetchPaged(`/api/v2/timesheets/on/${dateStr}?show_costs=true`) } catch { return [] } } -// Expand an array of timesheet objects into flat shift rows, filtering out leave and -// optionally filtering by location department IDs and enabled department IDs. -function expandShifts(timesheets, locationDeptIds, enabledDeptIds) { - const rows = [] - for (const t of timesheets) { - if (!Array.isArray(t.shifts)) continue - const userId = String(t.user_id) - for (const sh of t.shifts) { - if (sh.leave_request_id != null) continue // exclude leave accrual shifts - const deptId = String(sh.department_id ?? 'unknown') - if (locationDeptIds && !locationDeptIds.has(deptId)) continue // filter to this hotel - if (enabledDeptIds && !enabledDeptIds.includes(deptId)) continue - rows.push({ date: sh.date, userId, deptId, cost: parseFloat(sh.cost ?? 0) }) - } - } - return rows -} - export async function syncActuals(from, to) { const creds = await getWorkforceCreds() const locationId = creds.location_id ? String(creds.location_id) : null const enabledDeptIds = await getEnabledDeptIds() - // Fetch departments + users in parallel — departments used to filter to this hotel's depts - const [allDepts, userNameMap] = await Promise.all([ + // Fetch depts + users in parallel. + // Filter by user.report_department_id (employee's HOME location) not shift.department_id. + // This matches the PBI model: Hotels[Hotel] joined via user.report_department_id → + // department.location_id. Prevents shared depts (HR, Management, etc.) from pulling in + // cross-hotel staff who happen to clock into a No 4 department. + const [allDepts, allUsers] = await Promise.all([ wfFetchPaged('/api/v2/departments'), - getUserNameMap(), + wfFetchPaged('/api/v2/users'), ]) const deptNameMap = Object.fromEntries(allDepts.map(d => [String(d.id), d.name])) const locationDeptIds = locationId ? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id))) : null - // Fetch all dates in parallel — same pattern as PBI (no location param on API) - const dates = dateRange(from, to) - const perDay = await Promise.all(dates.map(fetchTimesheetsForDate)) + // user_id → { name, reportDeptId } + const userMap = Object.fromEntries(allUsers.map(u => [ + String(u.id), + { + name: u.name || `${u.legal_first_name || ''} ${u.legal_last_name || ''}`.trim() || `User ${u.id}`, + reportDeptId: u.report_department_id ? String(u.report_department_id) : null, + }, + ])) + // One fetch per 7-day step (PBI pattern) — each weekly timesheet encountered exactly once + const fetchDates = weeklyFetchDates(from, to) + const perFetch = await Promise.all(fetchDates.map(fetchTimesheetsForDate)) + + // Deduplicate timesheet records by id, then expand shifts + const seenTimesheetIds = new Set() const byDateDept = {} const byDateDeptEmp = {} - for (const timesheets of perDay) { - for (const { date, userId, deptId, cost } of expandShifts(timesheets, locationDeptIds, enabledDeptIds)) { - const key = `${date}:${deptId}` - if (!byDateDept[key]) { - byDateDept[key] = { - date, department_id: deptId, - department_name: deptNameMap[deptId] || deptId, - base_cost: 0, total_cost: 0, shift_count: 0, - } - } - byDateDept[key].base_cost += cost - byDateDept[key].total_cost += cost - byDateDept[key].shift_count += 1 + for (const timesheets of perFetch) { + for (const t of timesheets) { + if (seenTimesheetIds.has(t.id)) continue + seenTimesheetIds.add(t.id) + if (!Array.isArray(t.shifts)) continue - if (cost > 0) { - const empKey = `${date}:${deptId}:${userId}` - const empName = userNameMap[userId] || `Employee ${userId}` - if (!byDateDeptEmp[empKey]) { - byDateDeptEmp[empKey] = { - date, department_id: deptId, employee_id: userId, employee_name: empName, + const userId = String(t.user_id) + const user = userMap[userId] + + // Filter by employee's home location + if (locationDeptIds) { + if (!user?.reportDeptId || !locationDeptIds.has(user.reportDeptId)) continue + } + + for (const sh of t.shifts) { + if (sh.date < from || sh.date > to) continue // outside requested window + if (sh.leave_request_id != null) continue // exclude leave accrual shifts + + const deptId = String(sh.department_id ?? 'unknown') + if (enabledDeptIds && !enabledDeptIds.includes(deptId)) continue + + const cost = parseFloat(sh.cost ?? 0) + const key = `${sh.date}:${deptId}` + if (!byDateDept[key]) { + byDateDept[key] = { + date: sh.date, department_id: deptId, + department_name: deptNameMap[deptId] || deptId, base_cost: 0, total_cost: 0, shift_count: 0, } } - byDateDeptEmp[empKey].base_cost += cost - byDateDeptEmp[empKey].total_cost += cost - byDateDeptEmp[empKey].shift_count += 1 + byDateDept[key].base_cost += cost + byDateDept[key].total_cost += cost + byDateDept[key].shift_count += 1 + + if (cost > 0) { + const empKey = `${sh.date}:${deptId}:${userId}` + const empName = user?.name || `Employee ${userId}` + if (!byDateDeptEmp[empKey]) { + byDateDeptEmp[empKey] = { + date: sh.date, department_id: deptId, employee_id: userId, + employee_name: empName, base_cost: 0, total_cost: 0, shift_count: 0, + } + } + byDateDeptEmp[empKey].base_cost += cost + byDateDeptEmp[empKey].total_cost += cost + byDateDeptEmp[empKey].shift_count += 1 + } } } } @@ -314,28 +333,39 @@ async function fetchShiftsAndSummarise(path, locationDeptIds) { } } -// Fetch per-date timesheets for every day in [from, to] in parallel. -// Expands nested shifts[], filters leave, optionally filters by location dept IDs. -// This is the method used by PBI — no location param on the API call. -async function fetchTimesheetsByDay(from, to, locationDeptIds) { - const dates = dateRange(from, to) - const perDay = await Promise.all(dates.map(fetchTimesheetsForDate)) +// Fetch timesheets for [from, to] using PBI-matching method: +// - One fetch per 7-day step (avoid ~7x overcount from per-day fetching) +// - Deduplicate by timesheet.id +// - Filter to date range (sh.date) +// - Exclude leave (leave_request_id != null) +// - Filter by user.report_department_id (home location) not shift.department_id +async function fetchTimesheetsByDay(from, to, locationDeptIds, userMap) { + const fetchDates = weeklyFetchDates(from, to) + const perFetch = await Promise.all(fetchDates.map(fetchTimesheetsForDate)) + const seenIds = new Set() let totalShifts = 0, filteredShifts = 0, cost = 0 const byDept = {} let sampleShift = null - for (const timesheets of perDay) { - if (!Array.isArray(timesheets)) continue + for (const timesheets of perFetch) { for (const t of timesheets) { + if (seenIds.has(t.id)) continue + seenIds.add(t.id) if (!Array.isArray(t.shifts)) continue + + const userId = String(t.user_id) + const user = userMap ? userMap[userId] : null + const inLoc = !locationDeptIds || + (user?.reportDeptId && locationDeptIds.has(user.reportDeptId)) + if (locationDeptIds && !inLoc) continue // employee not based at this hotel + for (const sh of t.shifts) { + if (sh.date < from || sh.date > to) continue totalShifts++ - const isLeave = sh.leave_request_id != null - const deptId = String(sh.department_id ?? 'unknown') - const inLoc = !locationDeptIds || locationDeptIds.has(deptId) - if (isLeave || !inLoc) continue + if (sh.leave_request_id != null) continue filteredShifts++ + const deptId = String(sh.department_id ?? 'unknown') const shCost = parseFloat(sh.cost ?? 0) cost += shCost if (!sampleShift && shCost > 0) sampleShift = sh @@ -351,7 +381,8 @@ async function fetchTimesheetsByDay(from, to, locationDeptIds) { .sort((a, b) => b.cost - a.cost) return { - timesheetRecords: perDay.reduce((s, d) => s + (Array.isArray(d) ? d.length : 0), 0), + fetchDates, + uniqueTimesheets: seenIds.size, totalShifts, filteredShifts, cost: +cost.toFixed(2), @@ -368,34 +399,44 @@ export async function compareEndpoints(from, to) { const creds = await getWorkforceCreds() const locationId = creds.location_id ? String(creds.location_id) : null - // Build location dept IDs set for post-fetch filtering (PBI pattern) - const allDepts = await wfFetchPaged('/api/v2/departments') + // Fetch departments + users for filtering + const [allDepts, allUsers] = await Promise.all([ + wfFetchPaged('/api/v2/departments'), + wfFetchPaged('/api/v2/users'), + ]) const locationDeptIds = locationId ? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id))) : null + const userMap = Object.fromEntries(allUsers.map(u => [ + String(u.id), + { + name: u.name || `User ${u.id}`, + reportDeptId: u.report_department_id ? String(u.report_department_id) : null, + }, + ])) // A: Shifts with report_location_id (old sync method) const shiftsReportLoc = `/api/v2/shifts?from=${from}&to=${to}&show_costs=true&include_oncosts=true` + (locationId ? `&report_location_id=${locationId}` : '') - // B: Timesheets per-day, no API location filter, post-filtered to this hotel's depts - // This is the PBI method and should match WF "Timesheet exc. leave inc. allowances" + // B: Timesheets — weekly fetch step, dedup by timesheet.id, filter by user.report_department_id + // Mirrors exactly how PBI pulls and filters data. Target: WF "Timesheet exc.leave inc.allowances" const [a, b] = await Promise.all([ fetchShiftsAndSummarise(shiftsReportLoc, locationDeptIds), - fetchTimesheetsByDay(from, to, locationDeptIds), + fetchTimesheetsByDay(from, to, locationDeptIds, userMap), ]) return { from, to, location_id: locationId, location_dept_count: locationDeptIds ? locationDeptIds.size : 'all', - A_shifts_report_location_id: { - ...a, - note: 'Old sync method — shifts endpoint with report_location_id filter', - }, - B_timesheets_per_day_no_leave: { + no4_employee_count: userMap + ? Object.values(userMap).filter(u => u.reportDeptId && locationDeptIds?.has(u.reportDeptId)).length + : null, + A_shifts_report_location_id: { ...a, note: 'Old sync method — shifts endpoint with report_location_id filter' }, + B_timesheets_pbi_method: { ...b, - note: 'PBI method — timesheets/on/{date}, shifts expanded, leave excluded (leave_request_id!=null), filtered to this hotel\'s dept IDs. Target: WF Timesheet exc.leave inc.allowances', + note: 'PBI method — weekly fetch steps, dedup by timesheet.id, leave excluded, filtered by user.report_department_id. Target: WF Timesheet exc.leave inc.allowances', }, wf_report_reference: { note: 'Compare B.cost against WF "Timesheet exc. leave inc. allowances" figure',