feat(timeentry): reject single-bound PATCH inversion at the controller layer (#130 follow-up)#156
Merged
CryptoJones merged 1 commit intoMay 19, 2026
Conversation
…r layer (#130 follow-up) #130 added a zod `.refine()` on createTimeEntryBody and updateTimeEntryBody that rejects teEndedAt < teStartedAt when both bounds are in the request body. The PR called out the single-bound PATCH case as intentionally NOT covered there: a PATCH that supplies only teEndedAt (or only teStartedAt) merges against the row's existing value, which the schema layer can't see without a DB read. Said: > The single-bound PATCH case (only teEndedAt, validated against > the row's existing teStartedAt) can't be enforced at the schema > layer without a DB read. Leave that on the controller layer > for a follow-up. This is that follow-up. After the controller computes `mergedStart` / `mergedEnd` from the body + existing row, check whether the merged range is inverted; reject with 400 instead of silently dropping `teMinutes` to null (the previous behaviour, which the PR review confirmed had real-world bookkeeping cost: "clocked out before clocked in" rows look correct in their two timestamp columns but have an empty duration). Factored the check into a small `isInvertedRange(startedAt, endedAt)` helper, exposed via `_internals` for unit testing. Equality (zero-minute "Due on Receipt"-style entries) stays allowed. Unparseable input returns false from the helper because computeMinutes' own NaN guard already null-tags those rows; flagging them at this layer too would be double-counting a problem we'd 400 on elsewhere. Four new unit tests on the helper cover the four classes: missing-bound (false), happy-path including equality (false), inverted (true), unparseable (false). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CryptoJones
deleted the
feat/timeentry-controller-rejects-merged-inverted-range
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #155.
Summary
#130 fixed the both-bounds-in-body inversion case at the schema layer but explicitly punted on the single-bound PATCH: a request body with only
teEndedAt(or onlyteStartedAt) merges against the row's existing other bound, which the schema can't see without a DB read. Until now that path silently droppedteMinutesto null and stored a row whose clocked-out-before-clocked-in timestamps look correct but whose duration column is blank.Add the merge-then-check in the PATCH handler. Factor it into a small
isInvertedRange(startedAt, endedAt)helper exposed via_internalsfor unit testing. Equality stays allowed; unparseable input returns false (already null-tagged bycomputeMinutes).Test plan
npm run lint— cleannpm test— 608 passed (was 604 + 4 new helper-unit-tests), 15 skippedProudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/