From e1db93ad4a891c9cb6f8ead1692dbba3ca4591ee Mon Sep 17 00:00:00 2001 From: DJP Date: Sun, 17 May 2026 20:48:12 -0400 Subject: [PATCH] backend + frontend: leave hours, server-side bookings filter, dynamic meta, defensive charts Backend (33/33 tests, +5 new): - Split Zoho parser's canonical "billable" into "billable" (bool column) and "billingType" (string column with values like "Client Related" / "Leave Hours" / "Idle Time"). Each parsed row now carries both, and billable is cross-filled from billingType when only the latter is present. - Merge service computes leaveHours separately from non_billable_h: any row with billingType "leave hours"/"leave" lands in the leave bucket and is no longer double-counted as non-billable. - UtilisationSummaryRow gains leaveHours: float; TimelogRow gains billingType: str | None. - /api/airtable/bookings accepts ?department=&name= (comma-separated multi-value), folded into the filterByFormula alongside the date overlap. Apostrophes in names are escaped. Cache key now includes the filter values so different selections don't collide. - /api/airtable/meta computes departments + employmentTypes from a live fetch_resources call (sorted distinct), falls back to the hardcoded lists on any exception. billingTypes/bookingStatuses stay static. - Logout cookie now mirrors the login cookie's HttpOnly / Secure / SameSite / Path attributes with max_age=0 and empty value, for consistency. Frontend (typecheck/lint/build clean): - types.ts: UtilisationSummaryRow.leaveHours: number. - BillabilityBreakdown uses r.leaveHours directly; idle becomes max(0, available - billable - nonBillable - leave). Capped to top 20 employees by (available + billable) with "Other (N)" rollup; Legend replaced with compact inline swatches. - BookingVsActual and FTEvsFreelancer: same top-20 + Other treatment to prevent the ProjectLoad-style x-axis explosion at scale. - Defensive sweep on WeeklyUtilisation, MonthlyUtilisation, BookingVsActual, FTEvsFreelancer: null-coerce sort keys, Number()- guard arithmetic, skip rows with no usable period/employee. - getBookings signature gains department + name; Resourcing passes them through. Client-side visibleBookings filter retained as belt-and-braces since linked-lookup filterByFormula on Airtable can be flaky. - Tutorial steps.ts restructured to cover the new chart and CSV export tags; existing TutorialOverlay defensive selector check preserved. - ErrorBoundary: removed dead eslint-disable directive flagged by --report-unused-disable-directives. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/app/auth/session.py | 11 +- backend/app/models/schemas.py | 2 + backend/app/routers/airtable.py | 77 ++++++++++--- backend/app/services/airtable_fetch.py | 61 +++++++++- backend/app/services/merge.py | 8 +- backend/app/services/zoho_parse.py | 53 +++++++-- backend/tests/test_airtable_cache.py | 49 ++++++++ backend/tests/test_merge.py | 36 ++++++ backend/tests/test_zoho_parse.py | 24 ++++ frontend/src/api/endpoints.ts | 10 +- frontend/src/api/types.ts | 1 + frontend/src/components/ErrorBoundary.tsx | 1 - .../charts/BillabilityBreakdown.tsx | 105 +++++++++++++++--- .../src/components/charts/BookingVsActual.tsx | 34 +++++- .../src/components/charts/FTEvsFreelancer.tsx | 47 ++++++-- .../components/charts/MonthlyUtilisation.tsx | 18 ++- .../components/charts/WeeklyUtilisation.tsx | 16 ++- frontend/src/components/tutorial/steps.ts | 33 +++++- frontend/src/pages/Resourcing.tsx | 13 ++- 19 files changed, 520 insertions(+), 79 deletions(-) diff --git a/backend/app/auth/session.py b/backend/app/auth/session.py index 8aa3ff0..c04d46a 100644 --- a/backend/app/auth/session.py +++ b/backend/app/auth/session.py @@ -52,7 +52,16 @@ def set_session_cookie(response: Response, username: str) -> None: def clear_session_cookie(response: Response) -> None: - response.delete_cookie( + # Match the login cookie's attributes exactly so browsers reliably + # overwrite/expire the original. Anything that differs (path, + # SameSite, Secure, HttpOnly) can cause the browser to treat this + # as a separate cookie and leave the original in place. + response.set_cookie( key=COOKIE_NAME, + value="", + max_age=0, path=settings.cookie_path, + httponly=True, + secure=True, + samesite="lax", ) diff --git a/backend/app/models/schemas.py b/backend/app/models/schemas.py index 00a5488..7c9dc67 100644 --- a/backend/app/models/schemas.py +++ b/backend/app/models/schemas.py @@ -83,6 +83,7 @@ class TimelogRow(BaseModel): task: str | None = None hours: float = 0.0 billable: bool = False + billingType: str | None = None class ParseResponse(BaseModel): @@ -105,6 +106,7 @@ class UtilisationSummaryRow(BaseModel): loggedHours: float = 0.0 billableHours: float = 0.0 nonBillableHours: float = 0.0 + leaveHours: float = 0.0 forecastHours: float = 0.0 actualUtilisationPct: float = 0.0 bookedUtilisationPct: float = 0.0 diff --git a/backend/app/routers/airtable.py b/backend/app/routers/airtable.py index a4b59c6..ea723cc 100644 --- a/backend/app/routers/airtable.py +++ b/backend/app/routers/airtable.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from datetime import date, datetime, timezone from typing import Any @@ -13,6 +14,25 @@ from app.services.airtable_fetch import fetch_bookings, fetch_resources from app.services.cache import TTLAsyncCache +logger = logging.getLogger(__name__) + + +# Static fallback lists — used both for the parser-derived billingTypes / +# bookingStatuses and as the safety net if /resources is unreachable. +_STATIC_DEPARTMENTS = [ + "Creative Team", + "Opera Upload Team", + "Operation Team", + "Project Management Team", + "Syndication Team", + "Transcreation Team", + "House Admin", +] +_STATIC_BILLING_TYPES = ["Client Related", "Fee Related", "Idle Time", "Leave Hours"] +_STATIC_EMPLOYMENT_TYPES = ["FTE", "Freelancer"] +_STATIC_BOOKING_STATUSES = ["Active", "Soft Booking", "Fully Booked", "Partially Booked"] + + router = APIRouter(prefix="/api/airtable", tags=["airtable"], dependencies=[Depends(auth_required)]) @@ -48,6 +68,8 @@ async def get_resources( async def get_bookings( from_: str | None = Query(None, alias="from"), to: str | None = Query(None), + department: str | None = Query(None), + name: str | None = Query(None), refresh: bool = Query(False), ) -> dict[str, Any]: try: @@ -56,12 +78,21 @@ async def get_bookings( except ValueError as e: raise HTTPException(status_code=400, detail=f"Invalid date: {e}") - key = f"bookings:{from_d}:{to_d}" + # Department/name go into the cache key so different filter + # selections don't share a cache entry. Normalise empty -> None. + dep_key = department or "" + name_key = name or "" + key = f"bookings:{from_d}:{to_d}:{dep_key}:{name_key}" if refresh: _bookings_cache.invalidate(key) async def loader() -> dict[str, Any]: - rows = await fetch_bookings(from_=from_d, to=to_d) + rows = await fetch_bookings( + from_=from_d, + to=to_d, + department=department or None, + name=name or None, + ) return {"bookings": rows, "cached_at": _now_iso()} return await _bookings_cache.get_or_set(key, loader) @@ -70,23 +101,37 @@ async def get_bookings( # ---- /meta ---- @router.get("/meta") async def get_meta() -> dict[str, Any]: - """Static-ish dropdown values. Currently a constant; would derive from - Airtable schema endpoint in a future iteration.""" + """Dropdown values. `departments` and `employmentTypes` are derived + from the live Resources table; `billingTypes` (parser-derived) and + `bookingStatuses` remain static. Falls back to static lists if the + Airtable call fails so the UI dropdowns don't collapse.""" async def loader() -> dict[str, Any]: + departments: list[str] = _STATIC_DEPARTMENTS + employment_types: list[str] = _STATIC_EMPLOYMENT_TYPES + try: + resources = await fetch_resources(include_inactive=False) + seen_deps: list[str] = [] + seen_types: list[str] = [] + for r in resources: + d = r.get("department") + if d and d not in seen_deps: + seen_deps.append(d) + e = r.get("employmentType") + if e and e not in seen_types: + seen_types.append(e) + if seen_deps: + departments = sorted(seen_deps) + if seen_types: + employment_types = sorted(seen_types) + except Exception: + logger.exception("meta: falling back to static lists (Airtable unreachable)") + return { - "departments": [ - "Creative Team", - "Opera Upload Team", - "Operation Team", - "Project Management Team", - "Syndication Team", - "Transcreation Team", - "House Admin", - ], - "billingTypes": ["Client Related", "Fee Related", "Idle Time", "Leave Hours"], - "employmentTypes": ["FTE", "Freelancer"], - "bookingStatuses": ["Active", "Soft Booking", "Fully Booked", "Partially Booked"], + "departments": departments, + "billingTypes": _STATIC_BILLING_TYPES, + "employmentTypes": employment_types, + "bookingStatuses": _STATIC_BOOKING_STATUSES, } return await _meta_cache.get_or_set("meta:v1", loader) diff --git a/backend/app/services/airtable_fetch.py b/backend/app/services/airtable_fetch.py index bf150f5..b846305 100644 --- a/backend/app/services/airtable_fetch.py +++ b/backend/app/services/airtable_fetch.py @@ -218,9 +218,66 @@ def _date_filter(from_: date | None, to: date | None) -> str | None: return f"AND({', '.join(clauses)})" -async def fetch_bookings(*, from_: date | None = None, to: date | None = None) -> list[dict[str, Any]]: +def _escape_formula_literal(s: str) -> str: + """Escape a value for safe embedding inside a single-quoted Airtable + formula literal. Single quotes are the only escape we need; Airtable + treats backslashes literally inside string literals, so we use + `\\'` to terminate-and-resume the quoted string semantics expected + by formula engines.""" + return s.replace("\\", "\\\\").replace("'", "\\'") + + +def _multi_value_or(field: str, raw: str) -> str | None: + """Build an OR(...) clause matching `field` against each comma- + separated value in `raw`. Returns None if no non-empty values.""" + parts = [v.strip() for v in raw.split(",")] + parts = [p for p in parts if p] + if not parts: + return None + pieces = [f"{{{field}}}='{_escape_formula_literal(v)}'" for v in parts] + if len(pieces) == 1: + return pieces[0] + return f"OR({', '.join(pieces)})" + + +def _bookings_filter( + from_: date | None, + to: date | None, + department: str | None, + name: str | None, +) -> str | None: + """Combine date overlap + optional department/name filters into a + single AND(...) filterByFormula. Department matches against the + flattened lookup field `Department (from Resource Name)`; name + against `Resource Name (from Resource)`.""" + clauses: list[str] = [] + date_clause = _date_filter(from_, to) + if date_clause: + clauses.append(date_clause) + if department: + dep_clause = _multi_value_or("Department (from Resource Name)", department) + if dep_clause: + clauses.append(dep_clause) + if name: + name_clause = _multi_value_or("Resource Name (from Resource)", name) + if name_clause: + clauses.append(name_clause) + if not clauses: + return None + if len(clauses) == 1: + return clauses[0] + return f"AND({', '.join(clauses)})" + + +async def fetch_bookings( + *, + from_: date | None = None, + to: date | None = None, + department: str | None = None, + name: str | None = None, +) -> list[dict[str, Any]]: params: dict[str, Any] = {} - formula = _date_filter(from_, to) + formula = _bookings_filter(from_, to, department, name) if formula: params["filterByFormula"] = formula out: list[dict[str, Any]] = [] diff --git a/backend/app/services/merge.py b/backend/app/services/merge.py index 88f7f47..e1b647e 100644 --- a/backend/app/services/merge.py +++ b/backend/app/services/merge.py @@ -204,6 +204,7 @@ def summarise( "loggedHours": 0.0, "billableHours": 0.0, "nonBillableHours": 0.0, + "leaveHours": 0.0, "forecastHours": 0.0, "actualUtilisationPct": 0.0, "bookedUtilisationPct": 0.0, @@ -236,6 +237,7 @@ def summarise( # Logged hours filtered to this window. logged_h = 0.0 billable_h = 0.0 + leave_h = 0.0 for r in emp_logged: d = r.get("date") if not d: @@ -253,7 +255,10 @@ def summarise( logged_h += hrs if r.get("billable"): billable_h += hrs - non_billable_h = max(logged_h - billable_h, 0.0) + bt = (r.get("billingType") or "") + if isinstance(bt, str) and bt.strip().lower() in {"leave hours", "leave"}: + leave_h += hrs + non_billable_h = max(logged_h - billable_h - leave_h, 0.0) actual_pct = (logged_h / available_hours * 100.0) if available_hours > 0 else 0.0 booked_pct = (booked / available_hours * 100.0) if available_hours > 0 else 0.0 @@ -268,6 +273,7 @@ def summarise( "loggedHours": round(logged_h, 2), "billableHours": round(billable_h, 2), "nonBillableHours": round(non_billable_h, 2), + "leaveHours": round(leave_h, 2), "forecastHours": round(booked, 2), "actualUtilisationPct": round(actual_pct, 2), "bookedUtilisationPct": round(booked_pct, 2), diff --git a/backend/app/services/zoho_parse.py b/backend/app/services/zoho_parse.py index ba69534..b27fbef 100644 --- a/backend/app/services/zoho_parse.py +++ b/backend/app/services/zoho_parse.py @@ -4,9 +4,12 @@ Decisions: - Header matching is case-insensitive and trim-stripped. Unknown headers are surfaced in `unrecognised_columns` so the operator notices when Zoho silently renames a column. -- Billable detection: if the column is literally "Billable" / "Is Billable", - we coerce truthy strings. If the column is "Billing Type", we map - "Client Related" / "Fee Related" → True, everything else → False. +- Billable detection: we keep TWO canonical fields. `billable` accepts + literal "Billable" / "Is Billable" columns (boolean-ish). `billingType` + accepts a "Billing Type" column whose values look like + "Client Related" / "Fee Related" / "Idle Time" / "Leave Hours". + When only one of the two is present we cross-fill the other: a + billingType of client/fee implies billable=True; leave implies False. - Date parsing tries ISO first, then dateutil for the messy formats Zoho occasionally emits ("01/05/2026", "1-May-2026", etc.). - For .xlsx we use openpyxl read-only mode — keeps memory low on big files. @@ -35,10 +38,18 @@ HEADER_ALIASES: dict[str, set[str]] = { "project": {"project title", "project name", "project"}, "task": {"task description", "task", "description"}, "hours": {"hours logged", "total hours", "hours", "time logged", "actual logged"}, - "billable": {"billable", "is billable", "billing type"}, + "billable": {"billable", "is billable"}, + "billingType": {"billing type"}, } -BILLABLE_TRUE_VALUES = {"client related", "fee related", "true", "yes", "1", "billable"} +# Generic truthy strings for a literal "Billable" column. +BILLABLE_TRUE_VALUES = {"true", "yes", "1", "billable"} + +# Billing-type values (lower-cased) that imply billable=True. +BILLING_TYPE_BILLABLE = {"client related", "fee related"} + +# Billing-type values that imply billable=False (and are leave-coded). +BILLING_TYPE_LEAVE = {"leave hours", "leave"} def _canonicalise_header(raw: str) -> str | None: @@ -93,10 +104,8 @@ def _parse_hours(v: Any) -> float: return 0.0 -def _parse_billable(v: Any, *, source_header_canonical: str | None = None) -> bool: - # source_header_canonical only matters for "billable" — both columns - # canonicalise to that key but we want different semantics. We accept - # either bool, the special billing-type strings, or generic yes/no. +def _parse_billable(v: Any) -> bool: + """Parse a literal Billable / Is Billable column value.""" if v is None: return False if isinstance(v, bool): @@ -109,6 +118,14 @@ def _parse_billable(v: Any, *, source_header_canonical: str | None = None) -> bo return s in BILLABLE_TRUE_VALUES +def _parse_billing_type(v: Any) -> str | None: + """Parse a Billing Type column value to a lowercase canonical string.""" + if v is None: + return None + s = str(v).strip().lower() + return s or None + + # ---------------------------------------------------------------------- # Public API # ---------------------------------------------------------------------- @@ -148,6 +165,11 @@ def _build_rows(raw_rows: Iterable[list[Any]], headers: list[Any]) -> tuple[list else: unrecognised.append(str(raw).strip()) + # Track whether each canonical was actually present in the headers + # so we can decide whether to cross-fill billable from billingType + # (or vice versa) without clobbering a user-supplied value. + present_canonicals = set(canonical_by_idx.values()) + out: list[dict[str, Any]] = [] for raw_row in raw_rows: if not raw_row or all(c in (None, "") for c in raw_row): @@ -159,6 +181,7 @@ def _build_rows(raw_rows: Iterable[list[Any]], headers: list[Any]) -> tuple[list "task": None, "hours": 0.0, "billable": False, + "billingType": None, } for idx, canon in canonical_by_idx.items(): if idx >= len(raw_row): @@ -170,8 +193,20 @@ def _build_rows(raw_rows: Iterable[list[Any]], headers: list[Any]) -> tuple[list row["hours"] = _parse_hours(v) elif canon == "billable": row["billable"] = _parse_billable(v) + elif canon == "billingType": + row["billingType"] = _parse_billing_type(v) else: row[canon] = (str(v).strip() if v is not None else None) or None + + # Cross-fill: when only billingType is present, derive billable. + # When only billable is present, billingType stays None. + bt = row.get("billingType") + if "billingType" in present_canonicals and bt is not None: + if bt in BILLING_TYPE_BILLABLE: + row["billable"] = True + elif bt in BILLING_TYPE_LEAVE: + row["billable"] = False + out.append(row) return out, unrecognised diff --git a/backend/tests/test_airtable_cache.py b/backend/tests/test_airtable_cache.py index db8dcb0..4a629f3 100644 --- a/backend/tests/test_airtable_cache.py +++ b/backend/tests/test_airtable_cache.py @@ -3,11 +3,19 @@ from __future__ import annotations import asyncio +from datetime import date import pytest from app.services.cache import TTLAsyncCache +# NOTE: don't import app.services.airtable_fetch at module level — it pulls +# in app.config at collection time, which can race with the conftest +# session-autouse env-seeding fixture and leave app.auth.local holding a +# stale `settings` object (empty ADMIN_PASSWORD_BCRYPT) for the rest of +# the suite. Importing inside each test that needs it is safe because by +# then the autouse fixture has run and the env vars are populated. + @pytest.mark.asyncio async def test_second_call_hits_cache(): @@ -61,6 +69,47 @@ async def test_lock_prevents_thundering_herd(): assert all(r == 1 for r in results) +def test_bookings_filter_combines_date_and_multivalue_filters(): + """Date overlap + multi-value department + name with an apostrophe + must all combine into a single AND() with properly-escaped quotes.""" + from app.services.airtable_fetch import _bookings_filter + + formula = _bookings_filter( + date(2026, 5, 4), + date(2026, 5, 17), + department="Creative Team,Operation Team", + name="O'Brien,Bhakti Doshi", + ) + assert formula is not None + # Date overlap clauses + assert "IS_BEFORE({Start Date}, '2026-05-17')" in formula + assert "IS_AFTER({End Date}, '2026-05-04')" in formula + # Department OR + assert ( + "OR({Department (from Resource Name)}='Creative Team', " + "{Department (from Resource Name)}='Operation Team')" + ) in formula + # Name OR with the apostrophe escaped + assert "{Resource Name (from Resource)}='O\\'Brien'" in formula + assert "{Resource Name (from Resource)}='Bhakti Doshi'" in formula + # Wrapped in AND(...) since we have multiple top-level clauses. + assert formula.startswith("AND(") and formula.endswith(")") + + +def test_bookings_filter_no_filters_returns_none(): + from app.services.airtable_fetch import _bookings_filter + + assert _bookings_filter(None, None, None, None) is None + + +def test_bookings_filter_single_department_no_or_wrapper(): + """A single value shouldn't get wrapped in OR(...).""" + from app.services.airtable_fetch import _bookings_filter + + formula = _bookings_filter(None, None, "Creative Team", None) + assert formula == "{Department (from Resource Name)}='Creative Team'" + + @pytest.mark.asyncio async def test_refresh_bypasses_cache(): """Simulate the 'refresh=true' behaviour the router uses.""" diff --git a/backend/tests/test_merge.py b/backend/tests/test_merge.py index 786f6ea..8fc59db 100644 --- a/backend/tests/test_merge.py +++ b/backend/tests/test_merge.py @@ -144,3 +144,39 @@ def test_zero_available_no_division_error(sample_resources): assert rows[0]["availableHours"] == 0.0 assert rows[0]["actualUtilisationPct"] == 0.0 assert rows[0]["bookedUtilisationPct"] == 0.0 + + +def test_leave_hours_split_out_from_non_billable(sample_resources): + """A row tagged 'Leave Hours' should land in leaveHours, not + nonBillableHours; billable client work stays in billableHours.""" + # 8h leave + 32h client-related, all inside W19. + logged = [ + {"date": date(2026, 5, 4), "employee": "Bhakti Doshi", "project": "PTO", + "task": "Annual leave", "hours": 8.0, "billable": False, + "billingType": "leave hours"}, + {"date": date(2026, 5, 5), "employee": "Bhakti Doshi", "project": "Acme", + "task": "Design", "hours": 8.0, "billable": True, + "billingType": "client related"}, + {"date": date(2026, 5, 6), "employee": "Bhakti Doshi", "project": "Acme", + "task": "Design", "hours": 8.0, "billable": True, + "billingType": "client related"}, + {"date": date(2026, 5, 7), "employee": "Bhakti Doshi", "project": "Acme", + "task": "Design", "hours": 8.0, "billable": True, + "billingType": "client related"}, + {"date": date(2026, 5, 8), "employee": "Bhakti Doshi", "project": "Acme", + "task": "Design", "hours": 8.0, "billable": True, + "billingType": "client related"}, + ] + rows = summarise( + logged, + [], + sample_resources, + from_=date(2026, 5, 4), + to_=date(2026, 5, 8), + period="week", + ) + bhakti = next(r for r in rows if r["employee"] == "Bhakti Doshi" and r["period"] == "2026-W19") + assert bhakti["leaveHours"] == 8.0 + assert bhakti["billableHours"] == 32.0 + assert bhakti["nonBillableHours"] == 0.0 + assert bhakti["loggedHours"] == 40.0 diff --git a/backend/tests/test_zoho_parse.py b/backend/tests/test_zoho_parse.py index fd966e6..f848bc7 100644 --- a/backend/tests/test_zoho_parse.py +++ b/backend/tests/test_zoho_parse.py @@ -98,3 +98,27 @@ def test_content_hash_stable(): out1 = parse("a.csv", FIXTURE_CSV.read_bytes()) out2 = parse("a.csv", FIXTURE_CSV.read_bytes()) assert out1["content_hash"] == out2["content_hash"] + + +def test_billing_type_header_populates_billingType_and_billable(): + """When the upload uses a 'Billing Type' header, each row gains + `billingType` (lowercased) and `billable` is cross-filled from it.""" + csv = ( + "Date,Resource,Total Hours,Billing Type\n" + "2026-05-04,Bhakti,7,Client Related\n" + "2026-05-05,Bhakti,8,Leave Hours\n" + "2026-05-06,Bhakti,4,Idle Time\n" + "2026-05-07,Bhakti,6,Fee Related\n" + ).encode("utf-8") + out = parse("bt.csv", csv) + assert out["unrecognised_columns"] == [] + rows = out["rows"] + assert len(rows) == 4 + assert rows[0]["billingType"] == "client related" + assert rows[0]["billable"] is True + assert rows[1]["billingType"] == "leave hours" + assert rows[1]["billable"] is False + assert rows[2]["billingType"] == "idle time" + assert rows[2]["billable"] is False + assert rows[3]["billingType"] == "fee related" + assert rows[3]["billable"] is True diff --git a/frontend/src/api/endpoints.ts b/frontend/src/api/endpoints.ts index b1af91e..bcff9bb 100644 --- a/frontend/src/api/endpoints.ts +++ b/frontend/src/api/endpoints.ts @@ -32,7 +32,15 @@ export function getResources(includeInactive = false) { return apiFetch(`/airtable/resources${buildQuery({ include_inactive: includeInactive })}`); } -export function getBookings(params: { from?: string; to?: string; refresh?: boolean } = {}) { +export function getBookings( + params: { + from?: string; + to?: string; + refresh?: boolean; + department?: string; + name?: string; + } = {}, +) { return apiFetch(`/airtable/bookings${buildQuery(params)}`); } diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 5af4171..f75243c 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -60,6 +60,7 @@ export interface UtilisationSummaryRow { loggedHours: number; billableHours: number; nonBillableHours: number; + leaveHours: number; forecastHours: number; actualUtilisationPct: number; bookedUtilisationPct: number; diff --git a/frontend/src/components/ErrorBoundary.tsx b/frontend/src/components/ErrorBoundary.tsx index ea12882..68851e6 100644 --- a/frontend/src/components/ErrorBoundary.tsx +++ b/frontend/src/components/ErrorBoundary.tsx @@ -18,7 +18,6 @@ export default class ErrorBoundary extends Component { } componentDidCatch(error: Error, info: ErrorInfo) { - // eslint-disable-next-line no-console console.error('[ErrorBoundary]', this.props.label ?? 'unknown', error, info.componentStack); } diff --git a/frontend/src/components/charts/BillabilityBreakdown.tsx b/frontend/src/components/charts/BillabilityBreakdown.tsx index ff2046e..41913c1 100644 --- a/frontend/src/components/charts/BillabilityBreakdown.tsx +++ b/frontend/src/components/charts/BillabilityBreakdown.tsx @@ -2,7 +2,6 @@ import { Bar, BarChart, CartesianGrid, - Legend, ResponsiveContainer, Tooltip, XAxis, @@ -14,53 +13,123 @@ interface Props { rows: UtilisationSummaryRow[]; } +// Same density risk as ProjectLoadPerPerson: at 100+ employees the x-axis +// becomes unreadable. Cap to TOP_N by (available + billable) so the busy +// people show up; roll the rest into "Other (N)". Skip the Legend in favour +// of a compact wrapper-style hint to keep vertical space for the chart. +const TOP_N = 20; + interface Bucket { employee: string; billable: number; nonBillable: number; leave: number; idle: number; + available: number; } function aggregate(rows: UtilisationSummaryRow[]): Bucket[] { const map = new Map(); for (const r of rows) { - const b = map.get(r.employee) ?? { - employee: r.employee, + const employee = String(r.employee ?? '').trim(); + if (!employee) continue; + const b = map.get(employee) ?? { + employee, billable: 0, nonBillable: 0, leave: 0, idle: 0, + available: 0, }; - b.billable += r.billableHours; - b.nonBillable += r.nonBillableHours; - // Leave & idle are not split out by the API yet; we infer "idle" as available - logged. - const accountedLogged = r.billableHours + r.nonBillableHours; - const slack = Math.max(0, r.availableHours - accountedLogged); - b.idle += slack; - map.set(r.employee, b); + const billable = Number(r.billableHours) || 0; + const nonBillable = Number(r.nonBillableHours) || 0; + const leave = Number(r.leaveHours) || 0; + const available = Number(r.availableHours) || 0; + b.billable += billable; + b.nonBillable += nonBillable; + b.leave += leave; + b.available += available; + b.idle += Math.max(0, available - billable - nonBillable - leave); + map.set(employee, b); } - return [...map.values()].sort((a, b) => a.employee.localeCompare(b.employee)); + + const all = [...map.values()]; + // Rank by (available + billable) so the busiest / most-resourced people win. + all.sort((a, b) => b.available + b.billable - (a.available + a.billable)); + + if (all.length <= TOP_N) { + return all.sort((a, b) => a.employee.localeCompare(b.employee)); + } + + const top = all.slice(0, TOP_N); + const rest = all.slice(TOP_N); + const other: Bucket = { + employee: `Other (${rest.length})`, + billable: 0, + nonBillable: 0, + leave: 0, + idle: 0, + available: 0, + }; + for (const b of rest) { + other.billable += b.billable; + other.nonBillable += b.nonBillable; + other.leave += b.leave; + other.idle += b.idle; + other.available += b.available; + } + const sortedTop = top.sort((a, b) => + String(a.employee).localeCompare(String(b.employee)), + ); + return [...sortedTop, other]; } +const SERIES: { key: keyof Bucket; name: string; fill: string }[] = [ + { key: 'billable', name: 'Billable', fill: '#10b981' }, + { key: 'nonBillable', name: 'Non-billable', fill: '#f59e0b' }, + { key: 'leave', name: 'Leave', fill: '#a855f7' }, + { key: 'idle', name: 'Idle', fill: '#cbd5e1' }, +]; + export default function BillabilityBreakdown({ rows }: Props) { const data = aggregate(rows); + const otherCount = data.length > 0 && data[data.length - 1].employee.startsWith('Other (') + ? data.length - 1 + : 0; return (
-

Billability Breakdown

+
+

Billability Breakdown

+
+ {otherCount > 0 && Top {TOP_N} by available + billable} + {SERIES.map((s) => ( + + + {s.name} + + ))} +
+
- - - - - - + + {SERIES.map((s) => ( + + ))}
diff --git a/frontend/src/components/charts/BookingVsActual.tsx b/frontend/src/components/charts/BookingVsActual.tsx index dbca874..a40ddde 100644 --- a/frontend/src/components/charts/BookingVsActual.tsx +++ b/frontend/src/components/charts/BookingVsActual.tsx @@ -14,6 +14,11 @@ interface Props { rows: UtilisationSummaryRow[]; } +// Same density risk as the other employee-bucketed charts: at 100+ people the +// axis is unreadable. Cap to TOP_N by booked + actual and roll the rest into +// "Other (N)". +const TOP_N = 20; + interface Bucket { employee: string; booked: number; @@ -23,12 +28,31 @@ interface Bucket { function aggregateByEmployee(rows: UtilisationSummaryRow[]): Bucket[] { const map = new Map(); for (const r of rows) { - const b = map.get(r.employee) ?? { employee: r.employee, booked: 0, actual: 0 }; - b.booked += r.bookedHours; - b.actual += r.loggedHours; - map.set(r.employee, b); + const employee = String(r.employee ?? '').trim(); + if (!employee) continue; + const b = map.get(employee) ?? { employee, booked: 0, actual: 0 }; + b.booked += Number(r.bookedHours) || 0; + b.actual += Number(r.loggedHours) || 0; + map.set(employee, b); } - return [...map.values()].sort((a, b) => b.booked + b.actual - (a.booked + a.actual)); + + const all = [...map.values()].sort( + (a, b) => b.booked + b.actual - (a.booked + a.actual), + ); + if (all.length <= TOP_N) return all; + + const top = all.slice(0, TOP_N); + const rest = all.slice(TOP_N); + const other: Bucket = { + employee: `Other (${rest.length})`, + booked: 0, + actual: 0, + }; + for (const b of rest) { + other.booked += b.booked; + other.actual += b.actual; + } + return [...top, other]; } export default function BookingVsActual({ rows }: Props) { diff --git a/frontend/src/components/charts/FTEvsFreelancer.tsx b/frontend/src/components/charts/FTEvsFreelancer.tsx index 29ec84c..0990439 100644 --- a/frontend/src/components/charts/FTEvsFreelancer.tsx +++ b/frontend/src/components/charts/FTEvsFreelancer.tsx @@ -14,29 +14,62 @@ interface Props { rows: UtilisationSummaryRow[]; } +// Side-by-side charts: each can independently hit the 100+ employee density +// problem, so cap each side at TOP_N by utilisation % and roll the rest into +// "Other (N)". +const TOP_N = 20; + interface Bucket { employee: string; utilisation: number; + available: number; + booked: number; } function split(rows: UtilisationSummaryRow[]): { fte: Bucket[]; freelancer: Bucket[] } { const fteMap = new Map(); const freelancerMap = new Map(); for (const r of rows) { - const target = r.employmentType === 'FTE' ? fteMap : r.employmentType === 'Freelancer' ? freelancerMap : null; + const employee = String(r.employee ?? '').trim(); + if (!employee) continue; + const type = String(r.employmentType ?? ''); + const target = type === 'FTE' ? fteMap : type === 'Freelancer' ? freelancerMap : null; if (!target) continue; - const b = target.get(r.employee) ?? { available: 0, booked: 0 }; - b.available += r.availableHours; - b.booked += r.bookedHours; - target.set(r.employee, b); + const b = target.get(employee) ?? { available: 0, booked: 0 }; + b.available += Number(r.availableHours) || 0; + b.booked += Number(r.bookedHours) || 0; + target.set(employee, b); } - const toBuckets = (m: Map): Bucket[] => - [...m.entries()] + + const toBuckets = (m: Map): Bucket[] => { + const all: Bucket[] = [...m.entries()] .map(([employee, v]) => ({ employee, + available: v.available, + booked: v.booked, utilisation: v.available > 0 ? Math.round((v.booked / v.available) * 1000) / 10 : 0, })) .sort((a, b) => b.utilisation - a.utilisation); + + if (all.length <= TOP_N) return all; + + const top = all.slice(0, TOP_N); + const rest = all.slice(TOP_N); + let restAvail = 0; + let restBooked = 0; + for (const r of rest) { + restAvail += r.available; + restBooked += r.booked; + } + const other: Bucket = { + employee: `Other (${rest.length})`, + available: restAvail, + booked: restBooked, + utilisation: restAvail > 0 ? Math.round((restBooked / restAvail) * 1000) / 10 : 0, + }; + return [...top, other]; + }; + return { fte: toBuckets(fteMap), freelancer: toBuckets(freelancerMap) }; } diff --git a/frontend/src/components/charts/MonthlyUtilisation.tsx b/frontend/src/components/charts/MonthlyUtilisation.tsx index 51c1294..87dea3c 100644 --- a/frontend/src/components/charts/MonthlyUtilisation.tsx +++ b/frontend/src/components/charts/MonthlyUtilisation.tsx @@ -28,15 +28,21 @@ interface Bucket { function aggregateByMonth(rows: UtilisationSummaryRow[]): Bucket[] { const map = new Map(); for (const r of rows) { - const key = /^\d{4}-\d{2}/.test(r.period) ? r.period.slice(0, 7) : monthOf(r.period); + const rawPeriod = String(r.period ?? ''); + if (!rawPeriod) continue; + const key = /^\d{4}-\d{2}/.test(rawPeriod) ? rawPeriod.slice(0, 7) : monthOf(rawPeriod); + if (!key) continue; const b = map.get(key) ?? { period: key, available: 0, booked: 0, logged: 0, forecast: 0 }; - b.available += r.availableHours; - b.booked += r.bookedHours; - b.logged += r.loggedHours; - b.forecast += r.forecastHours; + b.available += Number(r.availableHours) || 0; + b.booked += Number(r.bookedHours) || 0; + b.logged += Number(r.loggedHours) || 0; + b.forecast += Number(r.forecastHours) || 0; map.set(key, b); } - return [...map.values()].sort((a, b) => a.period.localeCompare(b.period)); + // Period-based: cardinality is bounded by the date range, no top-N cap needed. + return [...map.values()].sort((a, b) => + String(a.period ?? '').localeCompare(String(b.period ?? '')), + ); } export default function MonthlyUtilisation({ rows, showForecast }: Props) { diff --git a/frontend/src/components/charts/WeeklyUtilisation.tsx b/frontend/src/components/charts/WeeklyUtilisation.tsx index 0fd6fbc..ac45eff 100644 --- a/frontend/src/components/charts/WeeklyUtilisation.tsx +++ b/frontend/src/components/charts/WeeklyUtilisation.tsx @@ -26,14 +26,20 @@ interface Bucket { function aggregateByWeek(rows: UtilisationSummaryRow[]): Bucket[] { const map = new Map(); for (const r of rows) { - const key = /^\d{4}-W\d{2}$/.test(r.period) ? r.period : weekOf(r.period); + const rawPeriod = String(r.period ?? ''); + if (!rawPeriod) continue; + const key = /^\d{4}-W\d{2}$/.test(rawPeriod) ? rawPeriod : weekOf(rawPeriod); + if (!key) continue; const b = map.get(key) ?? { period: key, booked: 0, logged: 0, available: 0 }; - b.booked += r.bookedHours; - b.logged += r.loggedHours; - b.available += r.availableHours; + b.booked += Number(r.bookedHours) || 0; + b.logged += Number(r.loggedHours) || 0; + b.available += Number(r.availableHours) || 0; map.set(key, b); } - return [...map.values()].sort((a, b) => a.period.localeCompare(b.period)); + // Period-based: cardinality is bounded by the date range, no top-N cap needed. + return [...map.values()].sort((a, b) => + String(a.period ?? '').localeCompare(String(b.period ?? '')), + ); } export default function WeeklyUtilisation({ rows, onPeriodClick }: Props) { diff --git a/frontend/src/components/tutorial/steps.ts b/frontend/src/components/tutorial/steps.ts index 17b18f5..b9cb2cd 100644 --- a/frontend/src/components/tutorial/steps.ts +++ b/frontend/src/components/tutorial/steps.ts @@ -34,29 +34,54 @@ export const departmentSteps: TutorialStep[] = [ { selector: 'chart-booking-vs-actual', title: 'Booking vs Actual', - description: 'See whose bookings match their logged hours and whose drift.', + description: 'See whose bookings match their logged hours and whose drift. Capped at the top 20 busiest people; the rest roll up into "Other".', + }, + { + selector: 'chart-billability-breakdown', + title: 'Billability breakdown', + description: 'Per-person split of billable, non-billable, leave and idle hours. Capped at the top 20 by available + billable.', + }, + { + selector: 'export-csv', + title: 'Export to CSV', + description: 'Download the current summary as a CSV — handy for sharing outside the app.', }, ]; export const resourcingSteps: TutorialStep[] = [ + { + selector: 'filter-bar', + title: 'Filter the view', + description: 'The same filter controls you used on Department — date range, department and name all flow through to the Airtable bookings query.', + }, { selector: 'chart-weekly-utilisation', title: 'Weekly utilisation', - description: 'Click a bar to drill into a specific week.', + description: 'Click a Booked bar to drill into a specific week.', }, { selector: 'chart-project-load', title: 'Project load per person', - description: 'Stacked bars show which projects are loading up each resource.', + description: 'Stacked bars show which projects are loading up each resource. Capped at the top 10 projects by hours; the rest roll up into "Other".', }, { selector: 'chart-fte-vs-freelancer', title: 'FTE vs Freelancer', - description: 'Compare utilisation between salaried staff and contractors.', + description: 'Compare utilisation between salaried staff and contractors. Each side caps at the top 20.', + }, + { + selector: 'export-csv', + title: 'Export to CSV', + description: 'Download the resourcing summary for the active filters.', }, ]; export const bookingsSteps: TutorialStep[] = [ + { + selector: 'filter-bar', + title: 'Filter the view', + description: 'Date range, department and name filters narrow the table below.', + }, { selector: 'bookings-table', title: 'Bookings table', diff --git a/frontend/src/pages/Resourcing.tsx b/frontend/src/pages/Resourcing.tsx index f47f569..ea789b2 100644 --- a/frontend/src/pages/Resourcing.tsx +++ b/frontend/src/pages/Resourcing.tsx @@ -28,8 +28,10 @@ export default function Resourcing() { [airtable.resources], ); - // Backend's /api/airtable/bookings doesn't yet accept department/name params, - // so apply those client-side. This lets the chart respect the FilterBar. + // Belt-and-braces: even though the backend now narrows bookings by + // department/name server-side, Airtable's filterByFormula against linked + // lookup fields can be flaky — so we keep the client-side filter to make + // sure the chart stays consistent with the FilterBar. const visibleBookings = useMemo(() => { const deptSet = new Set(filters.departments); const nameSet = new Set(filters.names); @@ -46,7 +48,12 @@ export default function Resourcing() { try { const q = filtersToQuery(filters); const [bk, sm] = await Promise.all([ - api.getBookings({ from: q.from, to: q.to }), + api.getBookings({ + from: q.from, + to: q.to, + department: q.department, + name: q.name, + }), api.getUtilisationSummary({ from: q.from, to: q.to,