Fix fatal error on servers without PHP calendar extension#229
Fix fatal error on servers without PHP calendar extension#229parhumm merged 3 commits intodevelopmentfrom
Conversation
Replace jddayofweek() with a pure PHP array-based day name lookup to avoid dependency on the optional PHP calendar extension. This fixes dashboard crashes on Synology NAS, Alpine containers, and minimal PHP builds. Fixes #228
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a private Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Helpers/DataBuckets.php (1)
107-108: Good fix – removes ext-calendar dependency and corrects Sunday handling.The array-based lookup correctly maps WordPress's
start_of_weekvalues (0=Sunday through 6=Saturday) to day names. This also fixes a latent bug in the original code wherejddayofweek($startOfWeek - 1, 1)would receive-1whenstart_of_week=0(Sunday), resulting in undefined behavior.Consider adding defensive bounds checking to prevent an undefined array key warning in PHP 8+ if the option value is ever corrupted:
🛡️ Optional defensive validation
$startOfWeek = (int) get_option('start_of_week', 1); +$startOfWeek = max(0, min(6, $startOfWeek)); // Clamp to valid range $dayNames = ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Helpers/DataBuckets.php` around lines 107 - 108, The current lookup uses $startOfWeek and $dayNames but lacks validation, so if get_option('start_of_week') returns an out-of-range or corrupted value you may hit an undefined array key; clamp or validate $startOfWeek to an integer in the 0..6 range before using it (e.g., coerce to int, then if <0 set to 0, if >6 set to 6 or fall back to a default) so the $dayNames lookup is always safe; update the code around the $startOfWeek and $dayNames usage to perform this bounds check and use the sanitized value for indexing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Helpers/DataBuckets.php`:
- Around line 107-108: The current lookup uses $startOfWeek and $dayNames but
lacks validation, so if get_option('start_of_week') returns an out-of-range or
corrupted value you may hit an undefined array key; clamp or validate
$startOfWeek to an integer in the 0..6 range before using it (e.g., coerce to
int, then if <0 set to 0, if >6 set to 6 or fall back to a default) so the
$dayNames lookup is always safe; update the code around the $startOfWeek and
$dayNames usage to perform this bounds check and use the sanitized value for
indexing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2856f5b-1687-4145-ab92-a56157207ee6
📒 Files selected for processing (1)
src/Helpers/DataBuckets.php
parhumm
left a comment
parhumm
left a comment
There was a problem hiding this comment.
Review Against Implementation Plan (#228)
Gap Found: Missing defensive fallback
Line 119: $dayNames[$startOfWeek] is missing the ?? 'Monday' null coalescing fallback from the implementation plan.
Risk: If start_of_week option is corrupted (outside 0–6 range), bare array access triggers:
- PHP 8.0–8.2:
Warning: Undefined array key+ empty string passed toDateTime::modify() - PHP 8.3+:
DateMalformedStringExceptionfatal error — the exact same class of crash this PR is fixing
Fix:
// Current (PR):
$start->modify('next ' . $dayNames[$startOfWeek]);
// Should be:
$start->modify('next ' . ($dayNames[$startOfWeek] ?? 'Monday'));The ?? 'Monday' fallback matches the default value in get_option('start_of_week', 1) and costs nothing in the happy path.
Other Notes
- PR template: "Fix" checkbox not checked
- CHANGELOG.md not updated (required by PR template)
Review based on implementation plan — verified across PHP 7.4–8.5, i18n safe, single call site.
Add ?? 'Monday' null coalescing fallback to $dayNames[$startOfWeek] to prevent Undefined array key warning (PHP 8.0+) and DateMalformedStringException (PHP 8.3+) when start_of_week option is corrupted to a value outside 0-6 range. Refs #228
Move inline $dayNames array to private const DAY_NAMES to avoid re-allocation on every initSeqWeek() call. Refs #228
E2E Test Results — Post-Fix VerificationBranch: Results: 24 passed, 1 skipped
Coverage
Verification Checklist Update
Commits Tested
Tested via |
parhumm
left a comment
parhumm
left a comment
There was a problem hiding this comment.
PR Review: APPROVE
Verdict: No security, performance, standards, or compatibility issues found.
Changes Summary
| Change | Line | Purpose |
|---|---|---|
private const DAY_NAMES |
class level | Pure-PHP day name lookup (replaces ext-calendar dependency) |
(int) get_option(...) |
109 | Type safety — WP stores options as strings |
self::DAY_NAMES[$startOfWeek] ?? 'Monday' |
120 | Defensive fallback for corrupted start_of_week values |
Review Checklist
- Security: No user input handling, no DB queries, no output — N/A
- Performance: Day names moved to
private const— zero per-call allocation - Standards:
(int)cast matchesChart.php:309pattern; null coalescing is PHP 7.0+ - Backward Compat: PHP 7.4+ safe; no hook/API changes; no schema changes
- i18n:
DateTime::modify()requires English day names — hardcoded English is correct, not a localization gap - Add-on Impact: No public method or hook changes — zero impact on Pro or third-party addons
Manual QA Checklist
Prerequisites: WordPress with Slimstat active, date range > 7 days on dashboard
- Navigate to Slimstat → Overview — charts render without errors
- Navigate to Slimstat → Access Log — page renders without errors
- Navigate to Slimstat → Reports — page renders without errors
- Navigate to Slimstat → World Map — page renders without errors
- Navigate to Slimstat → Settings — page renders without errors
- Change Settings → General → Week Starts On to Sunday (0) → reload Slimstat dashboard → charts render correctly
- Change Week Starts On to Monday (1) → reload → charts render correctly
- Change Week Starts On to Saturday (6) → reload → charts render correctly
- Set date range to Last 28 days → weekly chart buckets display correctly
- Set date range to Last 30 days → weekly chart buckets display correctly
- Set a custom date range spanning 2+ weeks → chart renders correctly
- Enable comparison chart toggle → no errors
- Check
wp-content/debug.log— no PHP warnings or errors from DataBuckets.php - Admin bar chart widget renders and updates via AJAX without errors
Review via wp-pr-review skill (jaan.to plugin) — E2E results: 24/24 passed
2 participants
Summary
Replace
jddayofweek()with a pure-PHP array-based day name lookup to avoid dependency on the optional PHPext-calendarextension. This fixes dashboard crashes on Synology NAS, Alpine containers, and minimal PHP builds.Closes #228
Changes
File:
src/Helpers/DataBuckets.php(int)cast toget_option('start_of_week', 1)for type safety (WP stores options as strings)$dayNamesarray — pure-PHP replacement forjddayofweek()jddayofweek($startOfWeek - 1, 1)with($dayNames[$startOfWeek] ?? 'Monday')— eliminatesext-calendardependency with safe fallback for corrupted valuesVerification
jddayofweek()fails on ALL PHP 7.1.20+ through 8.5.3 withoutext-calendarstart_of_weekvalues (0–6) produce identical day names to originaljddayofweek()callDateTime::modify()only accepts English day names — no i18n impact?? 'Monday'fallback preventsUndefined array keywarning on corrupted options (PHP 8.0+) andDateMalformedStringException(PHP 8.3+)Type of change
Summary by CodeRabbit