From fcea24b46a334bff1ce541634c7168732778732a Mon Sep 17 00:00:00 2001 From: jtricerolph Date: Thu, 23 Jul 2026 14:25:01 +0000 Subject: [PATCH] Fix timesheet sync: per-day fetch + shift.id dedup + dept_id filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key findings from diagnostic runs: - timesheets/on/{date} returns only that DATE's shifts (not the whole week), so weekly-step fetch only samples 6 days of a 30-day month - user.report_department_id filter too strict: only 32 staff have it set, missing many valid No 4 employees → £15k instead of £51k - Correct approach: per-day fetch (all 30 days), dedup by shift.id (robust against any duplicate timesheet records), filter by shift.department_id → location_id (matches WF "by Location and Team") Also includes inactive users (show_inactive=true) to catch staff who left mid-month but still have timesheets in the date range. Compare endpoint now shows baseCost (shift.cost) and oncostTotal (shift.cost_with_oncosts) so we can confirm which matches WF's £51k. Co-Authored-By: Claude Sonnet 4.6 --- backend/src/lib/workforce.js | 194 ++++++++++++++++------------------- 1 file changed, 90 insertions(+), 104 deletions(-) diff --git a/backend/src/lib/workforce.js b/backend/src/lib/workforce.js index f519a6c..6dd9615 100644 --- a/backend/src/lib/workforce.js +++ b/backend/src/lib/workforce.js @@ -69,10 +69,18 @@ async function getEnabledDeptIds() { } } -// 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). +// Build consecutive date list between from and to (inclusive) +function buildDateRange(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) } + return dates +} + +// Build weekly-step dates covering [from, to] — kept for compare diagnostics only. +// timesheets/on/{date} returns only that date's shifts (not the whole week), so +// weekly-step only samples 6 days of a 30-day month. Use buildDateRange for production. function weeklyFetchDates(from, to) { const dates = [] const limit = new Date(from + 'T00:00:00') @@ -98,58 +106,51 @@ export async function syncActuals(from, to) { const locationId = creds.location_id ? String(creds.location_id) : null const enabledDeptIds = await getEnabledDeptIds() - // 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. + // Fetch depts + users (inc. inactive — they may have timesheets in the date range) const [allDepts, allUsers] = await Promise.all([ wfFetchPaged('/api/v2/departments'), - wfFetchPaged('/api/v2/users'), + wfFetchPaged('/api/v2/users?show_inactive=true'), ]) const deptNameMap = Object.fromEntries(allDepts.map(d => [String(d.id), d.name])) + const userNameMap = Object.fromEntries(allUsers.map(u => [ + String(u.id), + u.name || `${u.legal_first_name || ''} ${u.legal_last_name || ''}`.trim() || `User ${u.id}`, + ])) + // Set of department IDs belonging to this location — used to filter shift.department_id. + // This matches how WF's "Cost by Location and Team" report groups costs: by the + // department the shift was worked in, not by where the employee is based. const locationDeptIds = locationId ? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id))) : null - // 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, - }, - ])) + // Fetch each day in parallel. The timesheets/on/{date} endpoint returns only that + // specific day's shift data (not the whole week), so per-day fetching is correct. + // Deduplicate by shift.id in case any shifts appear in more than one timesheet record. + const allDates = buildDateRange(from, to) + const perDay = await Promise.all(allDates.map(fetchTimesheetsForDate)) - // 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 seenShiftIds = new Set() const byDateDept = {} const byDateDeptEmp = {} - for (const timesheets of perFetch) { + for (const timesheets of perDay) { for (const t of timesheets) { - if (seenTimesheetIds.has(t.id)) continue - seenTimesheetIds.add(t.id) if (!Array.isArray(t.shifts)) continue - - 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 - } + const userId = String(t.user_id) + const empName = userNameMap[userId] || `Employee ${userId}` 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 + // Dedup by shift.id — robust against any overcount from overlapping timesheet records + const shiftId = String(sh.id) + if (seenShiftIds.has(shiftId)) continue + seenShiftIds.add(shiftId) + + if (sh.date < from || sh.date > to) continue + if (sh.leave_request_id != null) continue const deptId = String(sh.department_id ?? 'unknown') - if (enabledDeptIds && !enabledDeptIds.includes(deptId)) continue + if (locationDeptIds && !locationDeptIds.has(deptId)) continue + if (enabledDeptIds && !enabledDeptIds.includes(deptId)) continue const cost = parseFloat(sh.cost ?? 0) const key = `${sh.date}:${deptId}` @@ -165,8 +166,7 @@ export async function syncActuals(from, to) { byDateDept[key].shift_count += 1 if (cost > 0) { - const empKey = `${sh.date}:${deptId}:${userId}` - const empName = user?.name || `Employee ${userId}` + const empKey = `${sh.date}:${deptId}:${userId}` if (!byDateDeptEmp[empKey]) { byDateDeptEmp[empKey] = { date: sh.date, department_id: deptId, employee_id: userId, @@ -222,8 +222,9 @@ export async function syncScheduled(from, to) { let path = `/api/v2/schedules?from=${from}&to=${to}&show_costs=true&include_oncosts=true` if (locationId) path += `&location_id=${locationId}` - const schedules = await wfFetchPaged(path) - const deptNameMap = await getDeptNameMap() + const schedules = await wfFetchPaged(path) + const allDepts = await wfFetchPaged('/api/v2/departments') + const deptNameMap = Object.fromEntries(allDepts.map(d => [String(d.id), d.name])) const byDateDept = {} for (const s of schedules) { @@ -333,63 +334,65 @@ async function fetchShiftsAndSummarise(path, locationDeptIds) { } } -// 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)) +// Fetch all timesheets for [from, to] per day, dedup by shift.id, filter by shift.department_id. +// This is the "WF by Location and Team" method — costs by which dept the shift was worked in. +// Returns both shift.cost (base wages+allowances) and cost_with_oncosts (+ employer on-costs). +async function fetchTimesheetsByDay(from, to, locationDeptIds) { + const allDates = buildDateRange(from, to) + const perDay = await Promise.all(allDates.map(fetchTimesheetsForDate)) - const seenIds = new Set() - let totalShifts = 0, filteredShifts = 0, cost = 0 + const seenShiftIds = new Set() + let totalShifts = 0, leaveShifts = 0, filteredShifts = 0 + let baseCost = 0, oncostTotal = 0 const byDept = {} let sampleShift = null - for (const timesheets of perFetch) { + for (const timesheets of perDay) { 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) { + const shiftId = String(sh.id) + if (seenShiftIds.has(shiftId)) continue + seenShiftIds.add(shiftId) + if (sh.date < from || sh.date > to) continue - totalShifts++ - if (sh.leave_request_id != null) continue - filteredShifts++ const deptId = String(sh.department_id ?? 'unknown') - const shCost = parseFloat(sh.cost ?? 0) - cost += shCost + if (locationDeptIds && !locationDeptIds.has(deptId)) continue + totalShifts++ + + if (sh.leave_request_id != null) { leaveShifts++; continue } + filteredShifts++ + + const shCost = parseFloat(sh.cost ?? 0) + const shOncost = parseFloat(sh.cost_with_oncosts ?? sh.cost ?? 0) + baseCost += shCost + oncostTotal += shOncost if (!sampleShift && shCost > 0) sampleShift = sh - if (!byDept[deptId]) byDept[deptId] = { cost: 0, count: 0 } - byDept[deptId].cost += shCost + if (!byDept[deptId]) byDept[deptId] = { baseCost: 0, oncostTotal: 0, count: 0 } + byDept[deptId].baseCost += shCost + byDept[deptId].oncostTotal += shOncost byDept[deptId].count++ } } } const topDepts = Object.entries(byDept) - .map(([id, v]) => ({ id, cost: +v.cost.toFixed(2), count: v.count })) - .sort((a, b) => b.cost - a.cost) + .map(([id, v]) => ({ id, baseCost: +v.baseCost.toFixed(2), oncostTotal: +v.oncostTotal.toFixed(2), count: v.count })) + .sort((a, b) => b.baseCost - a.baseCost) return { - fetchDates, - uniqueTimesheets: seenIds.size, + dates: allDates.length, + uniqueShifts: seenShiftIds.size, totalShifts, + leaveShifts, filteredShifts, - cost: +cost.toFixed(2), + baseCost: +baseCost.toFixed(2), + oncostTotal: +oncostTotal.toFixed(2), topDepts: topDepts.slice(0, 15), sampleShiftFields: sampleShift ? Object.fromEntries(Object.entries(sampleShift).filter(([, v]) => v !== null && v !== undefined && v !== '')) : null, + note: 'baseCost = shift.cost (wages+allowances). oncostTotal = shift.cost_with_oncosts (+ employer pension + leave accrual). Target: WF Timesheet exc.leave inc.allowances', } } @@ -399,50 +402,33 @@ export async function compareEndpoints(from, to) { const creds = await getWorkforceCreds() const locationId = creds.location_id ? String(creds.location_id) : null - // Fetch departments + users for filtering - const [allDepts, allUsers] = await Promise.all([ - wfFetchPaged('/api/v2/departments'), - wfFetchPaged('/api/v2/users'), - ]) + const allDepts = await wfFetchPaged('/api/v2/departments') 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) + // A: Shifts endpoint with report_location_id (old method) const shiftsReportLoc = `/api/v2/shifts?from=${from}&to=${to}&show_costs=true&include_oncosts=true` + (locationId ? `&report_location_id=${locationId}` : '') - // 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" + // B: Per-day timesheets, dedup by shift.id, filter by shift.department_id → location. + // Both base cost and cost_with_oncosts reported. + // Target: match WF "Cost by Location and Team" Timesheet figure. const [a, b] = await Promise.all([ fetchShiftsAndSummarise(shiftsReportLoc, locationDeptIds), - fetchTimesheetsByDay(from, to, locationDeptIds, userMap), + fetchTimesheetsByDay(from, to, locationDeptIds), ]) return { from, to, - location_id: locationId, + location_id: locationId, location_dept_count: locationDeptIds ? locationDeptIds.size : 'all', - 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 — weekly fetch steps, dedup by timesheet.id, leave excluded, filtered by user.report_department_id. Target: WF Timesheet exc.leave inc.allowances', - }, + A_shifts_report_location_id: { ...a, note: 'Old method — shifts endpoint with report_location_id filter' }, + B_timesheets_per_day_dept_filter: b, wf_report_reference: { - note: 'Compare B.cost against WF "Timesheet exc. leave inc. allowances" figure', - scheduled_exc_leave: null, - timesheet_exc_leave_inc_all: null, - leave_accrual: null, + note: 'Compare B.baseCost or B.oncostTotal against WF "Timesheet exc. leave inc. allowances"', + timesheet_exc_leave: 51005.72, + leave_accrual: 8737.28, }, } }