Repository navigation
Say why a back-dated ongoing entry cannot be saved - #101
Merged
Merged
Conversation
An entry left open is a switch rather than history, so the server ends every open front at its start. Back-date that start to before somebody currently fronting began, and their end would land before their own beginning: ck_fronts_ended_after_started refuses it and the request answers 500, which surfaces as "Internal server error" with nothing to act on. 5xx keeps the generic wording on purpose, so there was no way to explain it after the fact. The sheet now checks before sending, names who it clashes with and when they started, and offers the two ways out: a later start, or an end time, which records it as history and leaves the live roster alone. The check is narrow on purpose. Editing never replaces anything, a closed entry is recorded without touching the live roster, and with the system's replace default off nothing is ended in the first place; all three skip it. Equal timestamps pass, since a front ending at the instant it began is a zero-length entry rather than a backwards one, which the constraint allows. A started_at this client cannot parse is left out rather than treated as a clash: the server is the authority on its own rows, and refusing a legitimate entry over a formatting disagreement is the worse error. Who is fronting comes from /v1/fronts/current when the sheet opens, not from the loaded history page. The page is paginated, so an open front is only probably on it, and a validation that is only probably right is worse than none.
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
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.
The follow-up left open by #97. That PR fixed adding a closed history entry; this one handles the "still ongoing" case it deliberately left alone.
The case
An entry left open is a switch, not history, so the server ends every open front at its start. Back-date that start to before somebody currently fronting began, and their end lands before their own beginning:
ck_fronts_ended_after_startedrefuses it and the request answers 500.The user sees "Internal server error". 5xx deliberately keeps the generic wording - server internals are not the user's problem and may say more than they should - so there is no explaining this after the fact. It has to be caught before the request goes out.
The check
The sheet now names who it clashes with and when they started, and offers the two ways out: a later start, or an end time, which records it as history and leaves the live roster alone.
It is narrow on purpose. Three cases skip it entirely, because none of them replace anything:
Two edge cases decided deliberately:
started_atis not treated as a clash. The server is the authority on its own rows, and refusing a legitimate entry over a formatting disagreement this client cannot read is the worse error.Where the data comes from
GET /v1/fronts/currentwhen the sheet opens, not the loaded history page.That was the open question in the backlog note: the history list is paginated, so an open front is only probably on it, and a validation that is only probably right is worse than none - it would wave through exactly the entry that 500s. Asked fresh on each open, since who is fronting changes between openings.
Tests
Six, against
openFrontsStartingAfter, which the rule was extracted into so it could be tested without a Compose harness: the breaking case, the safe case, equal timestamps, every clashing front reported rather than just the first, nothing open, and the unreadable timestamp.All clashing fronts are named rather than only the first, because fixing one and resubmitting into the next is a poor way to discover there were two.