feat: add pause/resume functionality for clock events and implement clock monitoring for breaks and auto clock-out#196
Conversation
…lock monitoring for breaks and auto clock-out Co-authored-by: Copilot <copilot@github.com>
…dels, services, and tests Co-authored-by: Copilot <copilot@github.com>
- Updated clock test cases to reflect changes in break segments to breaks. - Added new tests for shrinking overlapping break periods and manual break edits. - Enhanced TimesheetPage to manage editable breaks, including validation and rendering. - Refactored TimesheetRow to build a timeline of work and break segments. - Updated API to support breaks in clock event updates. - Added unit tests for TimesheetRow to ensure correct rendering and functionality. Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR upgrades the clock/break system to support pause/resume (break intervals), adds status reporting and break-aware timesheet calculations, and introduces a background monitor that sends break reminders and auto clocks users out at an 8-hour cap.
Changes:
- Added pause/resume + status APIs and client helpers, and extended the ClockEvent shape with break/paused metadata.
- Updated timesheet UI and calculations to display and edit break intervals, including summary “Break Hours”.
- Added a backend clock monitor service with periodic enforcement (3h/4h reminders, 8h auto clock-out), plus indexes and a migration for new fields.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/useClockToggle.ts | Adds pause/resume actions and loading state around new clock endpoints. |
| src/lib/timeUtils.ts | Updates active-session duration calculation to support accumulated time, pause state, and server-provided workSeconds. |
| src/lib/api.ts | Extends ClockEvent type and adds pause/resume/status endpoints and break-aware updateTimes payload. |
| src/features/timers/WorkPage.tsx | Disables timer controls while the user is on a clock break and shows “On Break” badge. |
| src/features/clock/TimesheetRow.tsx | Refactors timesheet rows into a work/break timeline rendering model. |
| src/features/clock/TimesheetRow.test.tsx | Adds unit tests validating work/break row rendering and edit action behavior. |
| src/features/clock/TimesheetPage.tsx | Adds break editing UI, break-aware duration math, and “Break Hours” summary. |
| src/features/clock/ClockPage.tsx | Adds Break/Resume button and paused-state labeling for the active session. |
| backend/tests/clock.test.ts | Adds integration tests for pause/resume/status, monitor reminders, auto clock-out, and break edits. |
| backend/src/services/timer.service.ts | Exposes helpers to close the running timer and fetch a timer session by id for break resume. |
| backend/src/services/clock.service.ts | Implements pause/resume/status, break normalization, and break-aware timesheet summaries. |
| backend/src/services/clock-monitor.service.ts | Adds periodic enforcement service (reminders + 8h auto clock-out). |
| backend/src/server.ts | Starts the clock monitor in non-test environments during bootstrap. |
| backend/src/routes/clock.ts | Adds /clock/pause, /clock/resume, and /clock/status endpoints + break payload support for updateTimes. |
| backend/src/models/clock.model.ts | Extends ClockEvent model with break, pause, notification, and auto clock-out fields. |
| backend/src/lib/ensure-indexes.ts | Adds new clockevents indexes to support monitor/enforcement queries. |
| backend/migrations/20260518_090000_add-clock-break-and-cap-fields.cjs | Backfills new clockevents fields and sets originalStartTime for existing records. |
| const breaks = toBreakIntervals(raw["breaks"]); | ||
|
|
||
| const now = Date.now(); | ||
| const workSeconds = getActiveWorkSeconds(e, now); |
| const prev = event.accumulatedTime ?? 0; | ||
| const elapsed = | ||
| typeof event.pausedAt === "number" ? 0 : getElapsedSeconds(event.startTime, now); | ||
| const finalSeconds = Math.min(MAX_WORK_SECONDS_PER_DAY, prev + elapsed); | ||
|
|
||
| // Close all open timer sessions for this user in a single updateMany | ||
| await timerService.closeAllForUser(userId, now); | ||
|
|
||
| const $set: Record<string, unknown> = { | ||
| endTime: now, | ||
| accumulatedTime: prev + elapsed, | ||
| accumulatedTime: finalSeconds, | ||
| pausedAt: null, | ||
| pauseStartedSessionId: null, | ||
| breaks: (() => { | ||
| const breaks = toBreakIntervals((event as unknown as Record<string, unknown>)["breaks"]); | ||
| if (typeof event.pausedAt !== "number") return breaks; | ||
| const openIdx = breaks.findIndex((b) => b.endTime === null); | ||
| if (openIdx === -1) return breaks; | ||
| const next = breaks.slice(); | ||
| next[openIdx] = { ...next[openIdx], endTime: now }; | ||
| return next; | ||
| })(), | ||
| ...(reason === "auto-8h" ? { autoClockedOutAt: now } : {}), | ||
| }; |
| const locked = await coll.updateOne( | ||
| { _id: event._id, endTime: null, autoClockedOutAt: null }, | ||
| { $set: { autoClockedOutAt: now } } | ||
| ); | ||
| if (locked.modifiedCount === 1) { | ||
| autoClockedOut += 1; | ||
| await clockService.stopWithReason(event.userId, event.teamId, "auto-8h"); | ||
| } |
| const coll = clockEventsCollection(); | ||
| const activeEvents = await coll | ||
| .find({ | ||
| endTime: null, | ||
| }) | ||
| .toArray(); | ||
|
|
|
@Dharp02 - I do not approve this P.R. It over complicates and bloats the simple ClockEvent in/out model. Break tracking is a good idea, but a proper data model is in order: e.g. ClockEvent stays the same as an in/out ledger. Break tracking exists as it's own ledger as an overlay. Mathematics in time reporting stays clean while augmented by breaks. Clean Approach (possible): It also has legal implications when capping/automating clocks: We need review/approval from @horner :
Here's a direct assessment of both questions: 1. Is this overkill?For a pure in/out time clock — yes, significantly. The changeset adds:
If the goal is just capturing shift start/end, this is 4–5x the necessary complexity. Pause/resume makes sense if unpaid meal breaks need to be tracked separately from work time — which is a legitimate compliance need — but the current implementation bundles a lot more on top of that. 2. US Regulatory ComplianceThere are real compliance concerns with the current approach: 🚨 The 8-Hour Cap Is the Biggest Problemconst MAX_WORK_SECONDS_PER_DAY = 8 * 60 * 60;This caps Break Classification Isn't DistinguishedFLSA has a hard rule:
The pause/resume implementation treats all breaks the same way — subtracting all paused time from State Law Adds More Complexity
|
|
@mfisher31 I’m still thinking through the architecture for this and trying to determine the cleanest long-term implementation approach. Thanks for calling out the compliance concerns as well — especially around FLSA and break classification. My intention with the 8-hour handling was more around the “user forgot to clock out” scenario rather than restricting actual worked hours. Even within the 8-hour cap flow, the user could still clock back in afterward to continue tracking additional hours if needed, so I wasn’t intending to block overtime or hide worked time. That said, I understand the concern with capping stored accumulated time itself. I can rework that behavior so the notification simply prompts the user to clock out instead of auto clocking them out automatically. The auto clock-out logic was mainly carried over because that behavior existed in the older TimeHarbor implementation that everyone had been using. |
- Introduced ClockBreak type to manage break entries with metadata. - Implemented auto-classification for breaks based on duration (rest vs meal). - Updated compute functions to accurately calculate work and break seconds. - Refactored clock service methods to utilize new break handling logic. - Removed deprecated fields from clock events and added migration scripts for data consistency. - Enhanced tests to cover new break classification logic and ensure accurate time calculations. - Added AdminTimesheetPanel to display timesheet information in the TeamsPage. Co-authored-by: Copilot <copilot@github.com>
… models - Added `clockbreaks` collection to store break intervals as separate documents. - Updated `ClockEvent` and `ClockBreak` interfaces to reflect the new structure. - Modified database indexes to support efficient querying of breaks. - Refactored clock service methods to handle breaks from the new collection. - Updated API responses to include breaks in the new format. - Adjusted migration scripts to extract existing breaks from `clockevents` to `clockbreaks`. - Revised tests to accommodate changes in break handling and notifications. Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (1)
backend/src/services/clock.service.ts:421
- This insert creates an open break without any DB-level uniqueness guarantee. If two pause requests race, you can end up with multiple
endTime: nullbreaks for the sameclockEventId, breakingisPaused/accounting. Enforce one-open-break-per-event at the DB layer (unique partial index) or make the write atomic.
await clockBreaksCollection().insertOne({
_id: new ObjectId(),
clockEventId: event._id.toHexString(),
startTime: now,
endTime: null,
…econds and paused state
… refactor routes for cleaner structure Co-authored-by: Copilot <copilot@github.com>
…ows-a-banner-until-acknowledged-or-resolved # Conflicts: # src/features/teams/TeamsPage.tsx
This pull request introduces major improvements to the backend database schema documentation and migration logic for clock event breaks. The changes include a comprehensive, up-to-date database reference, a migration to normalize and later extract clock event breaks into their own collection, and supporting migrations to backfill and clean up related fields. These updates clarify the data model for developers, improve query performance, and enable more flexible future development.
Database Documentation
DATABASE.mdas the single source of truth for the backend data model, including a mermaid diagram, field-level schema summaries, index catalog, and migration history. This will help all developers quickly understand and reason about the database structure.Clock Event Breaks Model Refactor
typeandclassificationSourcefields for embedded breaks, auto-classifying as"rest"or"meal"based on a 30-minute threshold.clockeventsdocuments, cleaning up legacy state.breaksarrays fromclockeventsinto a newclockbreakscollection, with full up/down support for moving between embedded and referenced models.ensure-indexes.tsto add indexes for the newclockbreakscollection, optimizing for open-break lookups and ordered retrieval per event.These changes modernize the time tracking schema, improve maintainability, and ensure the database documentation is always up to date.