fix: sync PlanText to handed-over shift on partial Accept#1565
Merged
renemadsen merged 1 commit intostablefrom May 7, 2026
Merged
fix: sync PlanText to handed-over shift on partial Accept#1565renemadsen merged 1 commit intostablefrom
renemadsen merged 1 commit intostablefrom
Conversation
Production bug on customer 855: handover Accept correctly persisted the moved shift to the receiver and cleared the sender's slot, but a downstream PlanRegistrationHelper.UpdatePlanRegistrationsInPeriod call ~2-9s later re-derived shift columns from each PR's stale PlanText, silently undoing the handover. DB evidence on request 41 (the most recent failed Accept): toPR v13 had the moved shift, then v14 zeroed it; fromPR v11 had slot 1 cleared, then v12 restored it from the still-present "6-10/0" PlanText segment. Fix: in AcceptAsync's partial-shift path, lift the matching segment verbatim from the sender's PlanText and insert it into the receiver's PlanText at the position matching SortShiftsByStart's column order. Then remove it from the sender. Lift-and-shift avoids any break-format round-trip loss (PlanRegistrationHelper.BreakTimeCalculator is a fixed string-key switch — only canonical decimal-hour strings round-trip). Match-by-start+end-only (PlanText is the source of truth; column break value can drift). Helpers added (private, non-static where they need _logger; static otherwise): - TryRemoveSegmentByStartEnd: returns (newPlanText, removedSegment). Match on (start, end) ignores break. - InsertSegmentSorted: keeps receiver's PlanText segment order in lockstep with the receiver's sorted shift columns so the next parser pass writes back into the same slot indices. - FormatShiftSegmentForFallback + FormatBreakAsCanonicalHours: used only when sender's PlanText didn't contain a matching segment. The break table mirrors PlanRegistrationHelper.BreakTimeCalculator's keys so the parser round-trips. Off-grid breaks emit decimal hours and would still be lost on round-trip — same limitation the existing system already has. - TryParseSegment + TryParseHm: parse one PlanText segment into minutes. Two new [Handover] log lines (receiver + sender) report the new PlanText and which segment was lifted (or "(none — fallback)" when the formatter fired). Full-day path (request.ShiftIndex == null) is unchanged — it already moves PlanText via direct copy. Plugin repo only — no edits to eform-timeplanning-base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where partial-shift handover Accept could be undone by later logic that re-derives shift columns from each PlanRegistration’s PlanText. The change keeps PlanText consistent with the shift-slot move performed during partial Accept.
Changes:
- In partial Accept, “lift-and-shift” the matching PlanText segment from sender → receiver, and remove it from the sender.
- Add fallback formatting (canonical break table) when no matching sender segment is found.
- Add helper methods for PlanText segment parsing/removal/insertion and additional
[Handover]log lines.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1550
to
+1558
| // Break is decimal hours in PlanText (1 = 60 min, 0.5 = 30 min, 0 = no break). | ||
| // Parse as double then multiply by 60 to compare against PlannedBreakOfShiftN | ||
| // columns, which are stored in minutes. | ||
| if (!double.TryParse(breakPart, System.Globalization.NumberStyles.Float, | ||
| System.Globalization.CultureInfo.InvariantCulture, | ||
| out var breakHours)) | ||
| { | ||
| return false; | ||
| } |
Comment on lines
+1526
to
+1529
| var normalized = segment.Replace(",", ".").Trim(); | ||
| var withBreakRegex = new System.Text.RegularExpressions.Regex(@"^(.*)-(.*)\/(.*)$"); | ||
| var noBreakRegex = new System.Text.RegularExpressions.Regex(@"^(.*)-(.*)$"); | ||
|
|
Comment on lines
+1486
to
+1488
| // Inserts `newSegment` into `planText` at the position that keeps segments | ||
| // sorted by start time. If `planText` is empty, returns just `newSegment`. | ||
| // Segments that fail to parse keep their original relative order at the end. |
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.
Summary
Customer 855 was still seeing the partial-shift handover Accept silently fail after PR #1564. DB evidence on request 41 showed the merged AcceptAsync did persist the moved shift to the receiver (v13) and clear the sender's slot (v11) — but a downstream
PlanRegistrationHelper.UpdatePlanRegistrationsInPeriodcall ~2-9 seconds later re-derived shift columns from each PR's stale PlanText, restoring the sender's slot from the unchanged"6-10/0"segment and zeroing the receiver's slot because its PlanText was empty.This PR keeps PlanText in sync with the shift moves on partial Accept.
Approach
Lift-and-shift the matching PlanText segment verbatim from sender to receiver:
fromPR.PlanTextwhose parsed(start, end)matches the moved shift; capture the verbatim string.toPR.PlanTextat the position keeping segments sorted by start time — so segment order matchesSortShiftsByStartcolumn order on the receiver.fromPR.PlanText. If that empties it, set to null (matches full-day branch convention).PlanRegistrationHelper.BreakTimeCalculator's string keys. Off-grid breaks (e.g. 7 min) still get lost on round-trip — same limitation the existing system has.Match-by-start+end-only because PlanText is the source of truth; the break value in PlanText can drift from
PlannedBreakOfShiftN.What changed
ContentHandoverService.cs— partial-shift branch inAcceptAsyncnow does the lift-and-shift; six new private helpers; two new[Handover]log lines.Why this fixes it
The downstream parser at
PlanRegistrationHelper.cs:600+writesPlanText.Split(';')[i]toShift[i+1]. After this PR, both PRs' PlanText is consistent with their shift columns immediately after Accept commits, so the next parser pass derives the same column values back instead of restoring stale ones.Test plan
🤖 Generated with Claude Code