fix: round up trial duration to avoid Carbon float truncation#193
Open
dan2k3k4 wants to merge 2 commits into
Open
fix: round up trial duration to avoid Carbon float truncation#193dan2k3k4 wants to merge 2 commits into
dan2k3k4 wants to merge 2 commits into
Conversation
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.
Fixes trials being set up to a day short when calculated from an end date, caused by Carbon 3 returning a fractional day count that was truncated.
Greptile Summary
This PR fixes a one-day shortfall in trial durations when
calculateAndSetTrialDatesFromEndDateis used, caused by Carbon 3'sdiffInDays()now returning a float that PHP's implicit int cast was silently truncating toward zero.PolydockAppInstance.php): wrapsdiffInDays()withceil()so any fractional day is rounded up to the next whole day, ensuring the trial always covers at least the requested end date.TrialDateCalculationTest.php): adds three cases — exact whole-day end dates, partial-day end dates (regression for the rounding bug), and a past end date (guards the null-reset path, now properly primed with a live trial before the call).Confidence Score: 5/5
Safe to merge — the change is a minimal one-line fix with correct rounding semantics and direct test coverage for the regression.
The fix correctly addresses Carbon 3's float return from diffInDays() by using ceil() instead of a truncating int cast. Behaviour for future dates (rounds up), exact whole-day dates (unchanged), and past dates (negative float still maps to ≤ 0, returning null) is all correct. The three new test cases exercise each path, including the previously-flagged null-reset path which is now properly primed.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant calculateAndSetTrialDatesFromEndDate participant Carbon participant calculateAndSetTrialDates Caller->>calculateAndSetTrialDatesFromEndDate: trialEndDateTime calculateAndSetTrialDatesFromEndDate->>Carbon: "now()->diffInDays(trialEndDateTime)" Carbon-->>calculateAndSetTrialDatesFromEndDate: float (e.g. 5.54) Note over calculateAndSetTrialDatesFromEndDate: ceil(float) → round up<br/>(was: int cast → truncate down) calculateAndSetTrialDatesFromEndDate->>calculateAndSetTrialDates: "durationDays = 6 (int)" alt "durationDays > 0" calculateAndSetTrialDates-->>Caller: "trial_ends_at = now() + 6 days" else "durationDays <= 0 (past date)" calculateAndSetTrialDates-->>Caller: "trial_ends_at = null" end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Caller participant calculateAndSetTrialDatesFromEndDate participant Carbon participant calculateAndSetTrialDates Caller->>calculateAndSetTrialDatesFromEndDate: trialEndDateTime calculateAndSetTrialDatesFromEndDate->>Carbon: "now()->diffInDays(trialEndDateTime)" Carbon-->>calculateAndSetTrialDatesFromEndDate: float (e.g. 5.54) Note over calculateAndSetTrialDatesFromEndDate: ceil(float) → round up<br/>(was: int cast → truncate down) calculateAndSetTrialDatesFromEndDate->>calculateAndSetTrialDates: "durationDays = 6 (int)" alt "durationDays > 0" calculateAndSetTrialDates-->>Caller: "trial_ends_at = now() + 6 days" else "durationDays <= 0 (past date)" calculateAndSetTrialDates-->>Caller: "trial_ends_at = null" endReviews (2): Last reviewed commit: "test: make past-date trial test exercise..." | Re-trigger Greptile