Store a goal where it happened, and draw half time across the timeline - #86
Merged
Merged
Conversation
A goal now carries the two facts a substitution already did — the half being played and the reading on the match clock — and the minute anyone sees is derived from them rather than frozen on the row when it was logged. Editing a match's duration no longer silently reinterprets goals already on file, and correcting a half's timings now moves its goals with its substitutions instead of leaving the two kinds of event disagreeing about the same timeline. Ordering follows: both kinds sort on elapsed seconds, which runs on across the break and needs no pair to keep a stoppage-time goal above the restart. That retires GameGoal.AdditionalMinute and MatchMinute.CompareTo, and leaves MatchMinute as the display it always looked like. The migration adds GamePeriodId and AtSeconds and drops AdditionalMinute, with no backfill: nothing left in an old row says whether a stored 37 was stoppage time or a minute typed in by hand, so those goals go on reading and sorting off Minute exactly as they do today. With the half on every event, the timeline can say where the break was — a dashed rule between the second half's entries and the first's, which is what issue #83 asked for on top of the model change. Closes #83 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD7SBD1CvRzK4Xrhx3dtf7
A minute somebody types on the result page is a scoreboard reading, and the scoreboard only agrees with the elapsed clock while the halves run to length. Ordering the timeline on (Minute - 1) * 60 quietly assumed they were the same scale, so on a match whose first half was whistled off three minutes long a second-half goal sorted ahead of one scored in first-half stoppage time — a wrong running score out of ScoreProgressionReport, and the goal drawn on the wrong side of the new half-time rule. MatchClockReport.ElapsedOf converts the typed minute back through the half's own timings, which is Build's arithmetic run the other way, and is now the only thing that produces an ordering key for a goal. GameGoal.TimelineSeconds is gone: it looked like it could answer this without the match, and it could not. Also drops the unused GamePeriod navigation on GameGoal — every reader resolves the half against the game already loaded — and corrects two claims in the migration that its own SQL contradicts: EF folds the column drop into the foreign key's table rebuild rather than running it last, and goals logged in stoppage time during the day AdditionalMinute existed do lose the overrun from their display. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD7SBD1CvRzK4Xrhx3dtf7
The migration dropped AdditionalMinute without rescuing the rows that had one, so a goal logged 30+2 during the day that column existed came out reading 30'. Those rows are not ambiguous: an overrun on a goal says outright that it was scored in stoppage time, so its half follows from the minute and its clock reading from that half's own kick-off — including when the half over-ran, which is exactly where a reconstruction that ignored the kick-off would land three minutes out. They read 30+2 again, which is the point, because 32 is the 32nd minute and that is two minutes into a second half. Goals with no overrun are still left alone. A stored 37 could be a minute typed in by hand on the result page, and guessing a half for it is how the frozen minute went wrong to begin with. GoalClockBackfillTests migrates a seeded database across the boundary and asserts what the app then shows rather than what landed in a column — the only migration here with a body worth testing, and it runs unattended against the live volume on the next deploy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XD7SBD1CvRzK4Xrhx3dtf7
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
2 participants
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.
A goal now carries the two facts a substitution already did — the half being
played and the reading on the match clock — and the minute anyone sees is
derived from them rather than frozen on the row when it was logged. Editing a
match's duration no longer silently reinterprets goals already on file, and
correcting a half's timings now moves its goals with its substitutions instead
of leaving the two kinds of event disagreeing about the same timeline.
Ordering follows: both kinds sort on elapsed seconds, which runs on across the
break and needs no pair to keep a stoppage-time goal above the restart. That
retires GameGoal.AdditionalMinute and MatchMinute.CompareTo, and leaves
MatchMinute as the display it always looked like.
The migration adds GamePeriodId and AtSeconds and drops AdditionalMinute, with
no backfill: nothing left in an old row says whether a stored 37 was stoppage
time or a minute typed in by hand, so those goals go on reading and sorting off
Minute exactly as they do today.
With the half on every event, the timeline can say where the break was — a
dashed rule between the second half's entries and the first's, which is what
issue #83 asked for on top of the model change.
Closes #83
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XD7SBD1CvRzK4Xrhx3dtf7