refactor(flex): delegate the chain to Microting.TimePlanningBase - #1700
Merged
Conversation
Bumps Microting.TimePlanningBase to 10.0.62 and deletes the plugin's own copies of the running flex-chain arithmetic and OneMinuteModeTimeline now that both live in the base package as FlexChain and Microting.TimePlanningBase.Infrastructure.Helpers.OneMinuteModeTimeline. PlanRegistrationHelper keeps its orchestration (UpdatePlanRegistrationsInPeriod, UpdatePlanRegistration, ReadBySiteAndDate, ...) and delegates arithmetic to FlexChain. Call sites across the plugin (services, GoogleSheetHelper) and the test project are repointed from PlanRegistrationHelper.X(...) to FlexChain.X(...) directly rather than left as forwarders, so there is a single source of truth for the chain. EnumerateShiftPauseStampPairs was kept in PlanRegistrationHelper (unlike its sibling private helpers) because it is still used by the unrelated EnumeratePauseStampPairs -> GetPauseIntervals -> ComputeTimeTrackingFields path, which was out of scope for this move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgEyDcnAEBcCF63RX2vm1k
There was a problem hiding this comment.
🟢 Approval recommended
The refactor cleanly repoints call sites to the base helpers with only a minor stale in-file reference remaining in a comment.
Pull request overview
Refactors the TimePlanning plugin to delegate flex-chain and one-minute-mode timeline logic to Microting.TimePlanningBase (v10.0.62), removing duplicated helper implementations from the plugin and repointing production/test call sites to the shared base helpers.
Changes:
- Bumped
Microting.TimePlanningBasedependency from10.0.61to10.0.62. - Replaced usages of plugin-local flex-chain helpers with
FlexChain.*from the base package across services and helpers. - Removed the plugin’s
OneMinuteModeTimelineimplementation and updated tests/usings to reference the base package’s version.
File summaries
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/TimePlanning.Pn.csproj | Bumps Microting.TimePlanningBase to 10.0.62 to consume shared flex-chain/timeline helpers. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs | Switches pause/flex-chain computations to FlexChain and keeps one-minute-mode behavior resolution. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs | Imports base helpers so OneMinuteModeTimeline references resolve to the base package. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs | Delegates flex-chain writes/clears to FlexChain and imports base helper namespace. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningFlexService/TimePlanningFlexService.cs | Delegates seconds/seed/clear operations to FlexChain and imports base helper namespace. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs | Deletes duplicated flex-chain/pause helpers and updates orchestration code to call FlexChain. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/OneMinuteModeTimeline.cs | Removes plugin-local timeline implementation (now consumed from base package). |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/GoogleSheetHelper.cs | Uses FlexChain.ClearSumFlexSeconds when mode-gated clearing is required. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/RunningFlexChainModeBoundaryTests.cs | Updates test imports to use base helper namespace. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanRegistrationHelperTests.cs | Repoints flex/pause/netto tests to FlexChain implementations from base package. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanningServiceMultiShiftTests.cs | Updates pause computations to FlexChain.ComputeShiftPauseSeconds. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/OneMinuteModeTimelineTests.cs | Switches tests to reference base OneMinuteModeTimeline. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/OneMinuteIntervalsEffectiveDateTests.cs | Switches timeline + seed fallback assertions to base helpers (OneMinuteModeTimeline, FlexChain). |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/CorruptedPauseIdRepairTests.cs | Repoints netto computation to FlexChain.ComputeNettoSecondsFromDateTimeShifts. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ComputeShiftPauseSecondsTests.cs | Repoints regression tests to FlexChain.ComputeShiftPauseSeconds. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -1041,7 +1041,7 @@ internal void ApplyRunningFlexChain( | |||
| // SumFlexStart when the seconds column is still 0 — otherwise | |||
| // the whole carried-forward balance is dropped at the head of | |||
| // the chain. See PlanRegistrationHelper.SecondsOrDecimalFallback. | |||
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.
Second of three releases implementing the flex chain continuity design. Requires base 10.0.62 (published).
What
The plugin deletes its own copies of the flex chain helpers and
OneMinuteModeTimelineand delegates to the base package. 124 insertions, 842 deletions — the plugin sheds far more than it gains, which is the shape a move should have.Removed from
PlanRegistrationHelperand now consumed fromFlexChain:SecondsOrDecimalFallback,SumFlexEndSecondsWithFallback,ClearSumFlexSeconds,ApplyNettoFlexChainDecimal, bothApplyNettoFlexChainSecondPrecisionoverloads,ComputeNettoSecondsFromDateTimeShifts,ComputeShiftPauseSeconds,AggregatePauseMinutes,GetShiftPauseOverrideMinutesand their private helpers.PlanRegistrationHelperkeeps all its orchestration.Call sites were repointed to
FlexChain.X(...)rather than left behind forwarders — a forwarder is a second name for one thing and invites exactly the divergence this work exists to end.Why it matters
The plugin and the background service each hold half a correct implementation. The service has the only unbounded, correctly-ordered forward cascade in the stack and is entirely mode-blind; the plugin has all the mode-awareness and no cascade reaching past today. This is the first half of ending that.
Behaviour neutrality
No decimal
SumFlexStart/SumFlexEnd/Flexvalue changes at any call site for any input. Review verified every moved method body byte-identical to the base package's copy, and confirmed no forwarders survive.The six fixtures that pin this behaviour —
PlanRegistrationHelperTests,OneMinuteIntervalsEffectiveDateTests,RunningFlexChainModeBoundaryTests,PlanRegistrationHelperDisplayParityTests,MobileFlexRecomputeAndCascadeTests,WorkingHoursDisplayParityTests— change only theirusinglines and static qualifiers. No assertion, argument or expected value moved anywhere. Three of them are untouched entirely.InternalsVisibleToneeded no change:OneMinuteModeTimeline's constructor andResolveByEffectiveDateare public in 10.0.62, precisely so the plugin's tests keep working once the type left the plugin's assembly.Known tech debt, deliberately not fixed here
EnumerateShiftPauseStampPairssurvives in the plugin as a duplicate of the base copy, because the base one isprivate. Review traced its callers:GetPauseIntervals/EnumeratePauseStampPairshave zero production callers and are exercised only by a reflection test. The clean fix is deleting that dead code — which removes the duplication with no base change — and is a separate cleanup, not this task's job.🤖 Generated with Claude Code
https://claude.ai/code/session_01TgEyDcnAEBcCF63RX2vm1k