Fix two root causes of timesheet figure overcount

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 <noreply@anthropic.com>
This commit is contained in:
jtricerolph 2026-07-23 14:11:14 +00:00
parent 2690ee149f
commit dd04b50fda

View file

@ -69,95 +69,114 @@ async function getEnabledDeptIds() {
} }
} }
async function getUserNameMap() { // Build fetch dates covering [from, to]: step backwards from 'to' in 7-day increments,
const all = await wfFetchPaged('/api/v2/users') // going one extra week before 'from' so weekly timesheets at the boundary are captured.
return Object.fromEntries( // Mirrors the PBI pattern (EndOfPeriod, EndOfPeriod-7, ...) — fetching one date per weekly
all.map(u => [String(u.id), u.name || `${u.legal_first_name || ''} ${u.legal_last_name || ''}`.trim() || `User ${u.id}`]) // timesheet period prevents counting the same record ~7x (one fetch per week, not per day).
) function weeklyFetchDates(from, to) {
}
// Build list of YYYY-MM-DD strings between from and to (inclusive)
function dateRange(from, to) {
const dates = [] const dates = []
const cur = new Date(from + 'T00:00:00') const limit = new Date(from + 'T00:00:00')
const end = new Date(to + 'T00:00:00') limit.setDate(limit.getDate() - 7)
while (cur <= end) { dates.push(cur.toISOString().slice(0, 10)); cur.setDate(cur.getDate() + 1) } 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 return dates
} }
// Fetch timesheets for a single date — no location param (mirrors PBI pattern). // 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) { async function fetchTimesheetsForDate(dateStr) {
try { return await wfFetchPaged(`/api/v2/timesheets/on/${dateStr}?show_costs=true`) } try { return await wfFetchPaged(`/api/v2/timesheets/on/${dateStr}?show_costs=true`) }
catch { return [] } 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) { export async function syncActuals(from, to) {
const creds = await getWorkforceCreds() const creds = await getWorkforceCreds()
const locationId = creds.location_id ? String(creds.location_id) : null const locationId = creds.location_id ? String(creds.location_id) : null
const enabledDeptIds = await getEnabledDeptIds() const enabledDeptIds = await getEnabledDeptIds()
// Fetch departments + users in parallel — departments used to filter to this hotel's depts // Fetch depts + users in parallel.
const [allDepts, userNameMap] = await Promise.all([ // 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'), wfFetchPaged('/api/v2/departments'),
getUserNameMap(), wfFetchPaged('/api/v2/users'),
]) ])
const deptNameMap = Object.fromEntries(allDepts.map(d => [String(d.id), d.name])) const deptNameMap = Object.fromEntries(allDepts.map(d => [String(d.id), d.name]))
const locationDeptIds = locationId const locationDeptIds = locationId
? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id))) ? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id)))
: null : null
// Fetch all dates in parallel — same pattern as PBI (no location param on API) // user_id → { name, reportDeptId }
const dates = dateRange(from, to) const userMap = Object.fromEntries(allUsers.map(u => [
const perDay = await Promise.all(dates.map(fetchTimesheetsForDate)) 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 byDateDept = {}
const byDateDeptEmp = {} const byDateDeptEmp = {}
for (const timesheets of perDay) { for (const timesheets of perFetch) {
for (const { date, userId, deptId, cost } of expandShifts(timesheets, locationDeptIds, enabledDeptIds)) { for (const t of timesheets) {
const key = `${date}:${deptId}` if (seenTimesheetIds.has(t.id)) continue
if (!byDateDept[key]) { seenTimesheetIds.add(t.id)
byDateDept[key] = { if (!Array.isArray(t.shifts)) continue
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
if (cost > 0) { const userId = String(t.user_id)
const empKey = `${date}:${deptId}:${userId}` const user = userMap[userId]
const empName = userNameMap[userId] || `Employee ${userId}`
if (!byDateDeptEmp[empKey]) { // Filter by employee's home location
byDateDeptEmp[empKey] = { if (locationDeptIds) {
date, department_id: deptId, employee_id: userId, employee_name: empName, 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, base_cost: 0, total_cost: 0, shift_count: 0,
} }
} }
byDateDeptEmp[empKey].base_cost += cost byDateDept[key].base_cost += cost
byDateDeptEmp[empKey].total_cost += cost byDateDept[key].total_cost += cost
byDateDeptEmp[empKey].shift_count += 1 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. // Fetch timesheets for [from, to] using PBI-matching method:
// Expands nested shifts[], filters leave, optionally filters by location dept IDs. // - One fetch per 7-day step (avoid ~7x overcount from per-day fetching)
// This is the method used by PBI — no location param on the API call. // - Deduplicate by timesheet.id
async function fetchTimesheetsByDay(from, to, locationDeptIds) { // - Filter to date range (sh.date)
const dates = dateRange(from, to) // - Exclude leave (leave_request_id != null)
const perDay = await Promise.all(dates.map(fetchTimesheetsForDate)) // - 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 let totalShifts = 0, filteredShifts = 0, cost = 0
const byDept = {} const byDept = {}
let sampleShift = null let sampleShift = null
for (const timesheets of perDay) { for (const timesheets of perFetch) {
if (!Array.isArray(timesheets)) continue
for (const t of timesheets) { for (const t of timesheets) {
if (seenIds.has(t.id)) continue
seenIds.add(t.id)
if (!Array.isArray(t.shifts)) continue 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) { for (const sh of t.shifts) {
if (sh.date < from || sh.date > to) continue
totalShifts++ totalShifts++
const isLeave = sh.leave_request_id != null if (sh.leave_request_id != null) continue
const deptId = String(sh.department_id ?? 'unknown')
const inLoc = !locationDeptIds || locationDeptIds.has(deptId)
if (isLeave || !inLoc) continue
filteredShifts++ filteredShifts++
const deptId = String(sh.department_id ?? 'unknown')
const shCost = parseFloat(sh.cost ?? 0) const shCost = parseFloat(sh.cost ?? 0)
cost += shCost cost += shCost
if (!sampleShift && shCost > 0) sampleShift = sh if (!sampleShift && shCost > 0) sampleShift = sh
@ -351,7 +381,8 @@ async function fetchTimesheetsByDay(from, to, locationDeptIds) {
.sort((a, b) => b.cost - a.cost) .sort((a, b) => b.cost - a.cost)
return { return {
timesheetRecords: perDay.reduce((s, d) => s + (Array.isArray(d) ? d.length : 0), 0), fetchDates,
uniqueTimesheets: seenIds.size,
totalShifts, totalShifts,
filteredShifts, filteredShifts,
cost: +cost.toFixed(2), cost: +cost.toFixed(2),
@ -368,34 +399,44 @@ export async function compareEndpoints(from, to) {
const creds = await getWorkforceCreds() const creds = await getWorkforceCreds()
const locationId = creds.location_id ? String(creds.location_id) : null const locationId = creds.location_id ? String(creds.location_id) : null
// Build location dept IDs set for post-fetch filtering (PBI pattern) // Fetch departments + users for filtering
const allDepts = await wfFetchPaged('/api/v2/departments') const [allDepts, allUsers] = await Promise.all([
wfFetchPaged('/api/v2/departments'),
wfFetchPaged('/api/v2/users'),
])
const locationDeptIds = locationId const locationDeptIds = locationId
? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id))) ? new Set(allDepts.filter(d => String(d.location_id) === locationId).map(d => String(d.id)))
: null : 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 with report_location_id (old sync method)
const shiftsReportLoc = `/api/v2/shifts?from=${from}&to=${to}&show_costs=true&include_oncosts=true` const shiftsReportLoc = `/api/v2/shifts?from=${from}&to=${to}&show_costs=true&include_oncosts=true`
+ (locationId ? `&report_location_id=${locationId}` : '') + (locationId ? `&report_location_id=${locationId}` : '')
// B: Timesheets per-day, no API location filter, post-filtered to this hotel's depts // B: Timesheets — weekly fetch step, dedup by timesheet.id, filter by user.report_department_id
// This is the PBI method and should match WF "Timesheet exc. leave inc. allowances" // Mirrors exactly how PBI pulls and filters data. Target: WF "Timesheet exc.leave inc.allowances"
const [a, b] = await Promise.all([ const [a, b] = await Promise.all([
fetchShiftsAndSummarise(shiftsReportLoc, locationDeptIds), fetchShiftsAndSummarise(shiftsReportLoc, locationDeptIds),
fetchTimesheetsByDay(from, to, locationDeptIds), fetchTimesheetsByDay(from, to, locationDeptIds, userMap),
]) ])
return { return {
from, to, from, to,
location_id: locationId, location_id: locationId,
location_dept_count: locationDeptIds ? locationDeptIds.size : 'all', location_dept_count: locationDeptIds ? locationDeptIds.size : 'all',
A_shifts_report_location_id: { no4_employee_count: userMap
...a, ? Object.values(userMap).filter(u => u.reportDeptId && locationDeptIds?.has(u.reportDeptId)).length
note: 'Old sync method — shifts endpoint with report_location_id filter', : null,
}, A_shifts_report_location_id: { ...a, note: 'Old sync method — shifts endpoint with report_location_id filter' },
B_timesheets_per_day_no_leave: { B_timesheets_pbi_method: {
...b, ...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: { wf_report_reference: {
note: 'Compare B.cost against WF "Timesheet exc. leave inc. allowances" figure', note: 'Compare B.cost against WF "Timesheet exc. leave inc. allowances" figure',