Add progress bars to client area #1924
Conversation
📝 WalkthroughWalkthroughAdds a ProgressBarColumn component and HasProgress trait, integrates progress bars into server list and entry blades (replacing textual CPU/Memory/Disk), updates Server::formatResource signature to accept precision, adds WARNING_THRESHOLD constant to ListServers, and caches Node system info/statistics. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as ListServers / Entry Blade
participant EntryCol as ServerEntryColumn
participant ProgressCol as ProgressBarColumn
participant Server as Server Model
participant BladeComp as progress-bar-column (Blade)
participant Node as Node Model / Cache
Page->>EntryCol: instantiate (setUp: default colors, thresholds)
Page->>ProgressCol: configure per-column closures (state,max,label,color,thresholds)
ProgressCol->>Server: evaluate closures to get current state/helper label
ProgressCol->>Node: fetch system/statistics (cached)
Node-->>ProgressCol: return cached data
ProgressCol->>ProgressCol: compute percentage & status, resolve color
ProgressCol->>BladeComp: render with props (state,max,percentage,color,label,ARIA)
BladeComp->>Browser: display progress bar UI (animate if danger), show helper label
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Models/Server.php (1)
485-511: Fix bitwise AND operator; verify base:3 parameter intent for server limits display.Line 501 uses bitwise AND (
&) instead of logical AND (&&). While this technically evaluates in a boolean context, it does not short-circuit and is semantically incorrect:if ($resourceAmount === 0 & $resourceType->isLimit()) {Should be:
if ($resourceAmount === 0 && $resourceType->isLimit()) {Additionally, line 510 is the only place in the codebase that calls
convert_bytes_to_readable()with an explicitbase: 3parameter, which forces all byte values to display in GiB/GB units. This differs from all other byte conversions, which omit the base parameter and auto-select units. Confirm that always formatting server limits in GB scale matches your intended UI output, or consider removing the parameter to let the helper function auto-detect the appropriate unit.app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
174-214: Remove the unusedgetResourceColormethod.Verification confirms this method is never called anywhere in the codebase. Since
ProgressBarColumnnow handles threshold-based coloring internally, this method is dead code and should be removed to reduce maintenance burden.Remove lines 174-214 from
app/Filament/App/Resources/Servers/Pages/ListServers.php.
🧹 Nitpick comments (2)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
5-180: Color/threshold API looks solid; consider aligningnormalizeColor()withProgressBarColumnfor consistency.The added threshold and color API (warning/danger percent, per‑status colors, and
getProgressColorForStatus) is well‑structured and should work cleanly with the progress‑bar views. One small improvement:normalizeColor()here is slightly different from the version inProgressBarColumn(alias handling vs. array handling order and how the first shade is selected). For long‑term maintainability and to avoid subtle divergences, it’s worth copying the exact implementation fromProgressBarColumn::normalizeColor()so both components resolve arrays and aliases identically.app/Filament/Components/Tables/Columns/ProgressBarColumn.php (1)
186-217: Consider extracting threshold defaults as class constants.The progress calculation methods are well-implemented with proper null safety and clamping. However, the hardcoded threshold defaults (0.7 and 0.9) on Lines 205-206 could optionally be extracted as class constants for better maintainability and discoverability.
If you'd like to make the defaults more explicit, consider this refactor:
class ProgressBarColumn extends Column { protected string $view = 'livewire.columns.progress-bar-column'; + private const DEFAULT_WARNING_THRESHOLD = 0.7; + private const DEFAULT_DANGER_THRESHOLD = 0.9; + // Accept int or float for max values protected int|float|Closure|null $maxValue = null;Then update the usage:
- $dangerThreshold = ($dangerPercent !== null ? $dangerPercent : 0.9) * 100; - $warningThreshold = ($warningPercent !== null ? $warningPercent : 0.7) * 100; + $dangerThreshold = ($dangerPercent !== null ? $dangerPercent : self::DEFAULT_DANGER_THRESHOLD) * 100; + $warningThreshold = ($warningPercent !== null ? $warningPercent : self::DEFAULT_WARNING_THRESHOLD) * 100;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
app/Filament/App/Resources/Servers/Pages/ListServers.php(5 hunks)app/Filament/Components/Tables/Columns/ProgressBarColumn.php(1 hunks)app/Filament/Components/Tables/Columns/ServerEntryColumn.php(1 hunks)app/Models/Server.php(2 hunks)resources/views/livewire/columns/progress-bar-column.blade.php(1 hunks)resources/views/livewire/server-entry-placeholder.blade.php(2 hunks)resources/views/livewire/server-entry.blade.php(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/Models/Server.php (1)
app/helpers.php (2)
format_number(108-116)convert_bytes_to_readable(36-49)
app/Filament/App/Resources/Servers/Pages/ListServers.php (3)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (4)
ProgressBarColumn(10-261)warningThresholdPercent(61-66)dangerThresholdPercent(73-78)maxValue(49-54)app/Filament/Components/Tables/Columns/ServerEntryColumn.php (2)
warningThresholdPercent(42-47)dangerThresholdPercent(54-59)app/Models/Server.php (3)
Server(134-525)formatResource(485-511)retrieveResources(475-483)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (9)
warningThresholdPercent(61-66)dangerThresholdPercent(73-78)dangerColor(88-93)warningColor(106-111)color(124-129)setUp(38-47)getColor(134-137)normalizeColor(143-167)getWarningColor(116-119)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (1)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (12)
warningThresholdPercent(42-47)dangerThresholdPercent(54-59)dangerColor(69-74)warningColor(87-92)color(105-110)setUp(33-40)getColor(115-118)getWarningThresholdPercent(49-52)getDangerThresholdPercent(61-64)getDangerColor(79-82)normalizeColor(126-154)getWarningColor(97-100)
🔇 Additional comments (11)
resources/views/livewire/server-entry-placeholder.blade.php (1)
3-4: Placeholder progress bars are wired correctly.The placeholder now reuses the same progress‑bar partial and ServerEntryColumn color logic as the live view, with safe 0‑valued state, correct max fallbacks, and sensible labels (including ∞ for unlimited limits). This keeps the skeleton UI visually consistent without introducing edge‑case risks.
Also applies to: 45-61, 64-80, 83-99
app/Filament/App/Resources/Servers/Pages/ListServers.php (3)
38-42: LGTM!The threshold constants are well-defined and appropriately scoped for reuse across grid and table columns.
57-60: LGTM!The threshold configuration for the grid columns correctly uses the class constants.
76-76: LGTM!The precision parameter enhances the uptime display accuracy.
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (7)
10-36: LGTM!The property declarations are well-typed and properly documented with PHPDoc for complex array types.
38-47: LGTM!The default color initialization and helper label setup are appropriate and follow framework conventions.
49-59: LGTM!The max value methods follow the fluent API pattern correctly and handle closure evaluation properly.
61-83: LGTM!The threshold methods are implemented consistently with proper closure evaluation.
85-137: LGTM!The color methods follow a consistent pattern with proper evaluation and normalization.
169-184: LGTM!The helper label methods correctly provide context (state and percentage) during evaluation and handle null results safely.
219-260: LGTM!The label and color resolution methods are well-implemented with appropriate fallback logic to ensure non-null return values as documented.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
resources/views/livewire/server-entry.blade.php (3)
67-87: Add defensive array access for node stats to prevent notices.Line 70 directly accesses
$serverNodeSystemInfo['cpu_count']without null coalescing. If this key is missing from the node data, PHP will emit an undefined array key notice, and the fallback calculation will becomenull * 100 = 0.Apply defensive access:
- $cpuMax = \App\Enums\ServerResourceType::CPULimit->getResourceAmount($server) === 0 ? ($serverNodeSystemInfo['cpu_count'] * 100) : \App\Enums\ServerResourceType::CPULimit->getResourceAmount($server); + $cpuMax = \App\Enums\ServerResourceType::CPULimit->getResourceAmount($server) === 0 ? (($serverNodeSystemInfo['cpu_count'] ?? 0) * 100) : \App\Enums\ServerResourceType::CPULimit->getResourceAmount($server);
111-131: Add defensive array access for disk stats to prevent notices.Line 114 directly accesses
$serverNodeStatistics['disk_total']without null coalescing. If this key is missing, PHP will emit an undefined array key notice.Apply defensive access:
- $diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['disk_total'] : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server); + $diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['disk_total'] ?? 0) : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server);
89-109: Add defensive array access for memory stats to prevent notices.Line 92 directly accesses
$serverNodeStatistics['memory_total']without null coalescing. If this key is missing, PHP will emit an undefined array key notice.Apply defensive access:
- $memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['memory_total'] : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server); + $memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['memory_total'] ?? 0) : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server);
🧹 Nitpick comments (1)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
126-154: Refactor normalizeColor() to match ProgressBarColumn's clearer logic flow.The current implementation works but is harder to follow than ProgressBarColumn's approach. Line 132 extracts array values for alias checking, then Line 147 checks is_array again. This creates a confusing execution path.
For consistency and clarity, match ProgressBarColumn's logic flow:
protected function normalizeColor(string|array|null $color): string|array|null { if ($color === null) { return null; } - $lower = strtolower(trim(is_array($color) ? (string) ($color[500] ?? '') : (string) $color)); + if (is_array($color)) { + $first = $color[500] ?? reset($color); + + return Color::convertToRgb((string) $first); + } + + $lower = strtolower(trim($color)); $aliases = [ 'danger' => FilamentColor::getColor('danger'), 'warning' => FilamentColor::getColor('warning'), 'primary' => FilamentColor::getColor('primary'), ]; if (isset($aliases[$lower])) { $resolved = $aliases[$lower]; $resolvedArray = (array) $resolved; $value = reset($resolvedArray); return Color::convertToRgb((string) $value); } - if (is_array($color)) { - $first = $color[500] ?? reset($color); - - return Color::convertToRgb((string) $first); - } - return Color::convertToRgb($color); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/Filament/App/Resources/Servers/Pages/ListServers.php(5 hunks)app/Filament/Components/Tables/Columns/ProgressBarColumn.php(1 hunks)app/Filament/Components/Tables/Columns/ServerEntryColumn.php(1 hunks)resources/views/livewire/columns/progress-bar-column.blade.php(1 hunks)resources/views/livewire/server-entry.blade.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/views/livewire/columns/progress-bar-column.blade.php
🧰 Additional context used
🧬 Code graph analysis (2)
app/Filament/App/Resources/Servers/Pages/ListServers.php (3)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (5)
ProgressBarColumn(10-265)warningThresholdPercent(61-66)dangerThresholdPercent(73-78)maxValue(49-54)helperLabel(173-178)app/Filament/Components/Tables/Columns/ServerEntryColumn.php (2)
warningThresholdPercent(42-47)dangerThresholdPercent(54-59)app/Models/Server.php (3)
Server(134-525)formatResource(485-511)retrieveResources(475-483)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (10)
warningThresholdPercent(61-66)dangerThresholdPercent(73-78)dangerColor(88-93)warningColor(106-111)color(124-129)setUp(38-47)getColor(134-137)getDangerColor(98-101)normalizeColor(143-171)getWarningColor(116-119)
🔇 Additional comments (22)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (6)
5-7: LGTM!The new imports (Closure, Color, FilamentColor) are appropriate for the threshold and color management functionality being added.
14-31: LGTM!The property declarations for thresholds and colors are well-structured and consistent with the ProgressBarColumn pattern. The PHPDoc annotations clearly document the array shapes.
33-40: LGTM!The setUp() method correctly initializes default colors and follows the same pattern as ProgressBarColumn, ensuring consistent behavior across components.
42-64: LGTM!The threshold setters and getters are correctly implemented with fluent interface support and proper Closure evaluation.
66-118: LGTM!The color setters and getters are well-implemented with proper PHPDoc annotations, fluent interface support, and normalization of evaluated colors.
156-180: LGTM!The getProgressColorForStatus() method correctly resolves colors based on status with appropriate fallbacks, mirroring ProgressBarColumn::getProgressColor() for consistent behavior across components.
app/Filament/App/Resources/Servers/Pages/ListServers.php (6)
8-8: LGTM!The import of ProgressBarColumn and the addition of WARNING_THRESHOLD constant are appropriate. Making the thresholds public constants allows them to be reused across components.
Also applies to: 38-42
58-59: LGTM!The threshold configuration on ServerEntryColumn correctly wires the class constants, ensuring consistent threshold behavior across grid and table views.
76-76: LGTM!The addition of precision parameter (2) for the uptime tooltip provides more detailed time information, improving user experience.
90-96: LGTM!The CPU ProgressBarColumn is correctly configured with appropriate null coalescing (
?? 0), reasonable maxValue logic for unlimited cases, and proper precision for CPU display.
97-103: LGTM!The Memory ProgressBarColumn correctly handles missing data with
?? 0and properly converts memory limit to bytes for consistent units with the state value.
104-110: LGTM!The Disk ProgressBarColumn correctly handles missing data with
?? 0and properly converts disk limit to bytes for consistent units with the state value.app/Filament/Components/Tables/Columns/ProgressBarColumn.php (10)
1-36: LGTM!The class structure and property declarations are well-designed with appropriate types and nullability, providing a solid foundation for the progress bar functionality.
38-47: LGTM!The setUp() method appropriately initializes default colors and provides a sensible default helperLabel implementation.
49-59: LGTM!The maxValue setter and getter are correctly implemented with flexible type support (int, float, Closure) and proper evaluation.
61-83: LGTM!The threshold setters and getters follow a consistent pattern with proper Closure evaluation and fluent interface support.
85-137: LGTM!The color setters and getters are consistently implemented across all three color properties (danger, warning, primary) with proper normalization.
143-171: LGTM!The normalizeColor() method correctly handles null values (addressing previous review feedback) and follows a clean, logical flow: null check → array handling → alias resolution → RGB conversion.
173-188: LGTM!The helperLabel implementation provides flexibility by passing both state and percentage to the closure, enabling various label formatting options.
190-200: LGTM!The progress percentage calculation correctly handles edge cases (null values, zero max) and bounds the result between 0 and 100.
202-221: LGTM!The status determination logic correctly applies configurable thresholds with sensible defaults (0.9 for danger, 0.7 for warning) and evaluates them in the right order.
223-264: LGTM!Both methods are well-implemented:
- getProgressLabel() provides a flexible labeling system with sensible fallback
- getProgressColor() guarantees a non-null color through a robust fallback chain (status-based → primary → gray)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
38-42: Clarify threshold semantics (ratio vs percent) for consistency.You’re using
WARNING_THRESHOLD = 0.7/DANGER_THRESHOLD = 0.9both ingetResourceColor()(as ratios of the limit) and inwarningThresholdPercent()/dangerThresholdPercent()onServerEntryColumnandProgressBarColumn. Given the method names end in*Percent, it’d be good to confirm those APIs expect 0–1 ratios (70% as0.7) and not 0–100 values.If they expect 0–100, these values will effectively be 0.7% / 0.9% thresholds. Either:
- Keep them as ratios and ensure the column classes also treat them as ratios, or
- Change the constants to
70.0/90.0(and possibly rename to*_PERCENT) if the column logic works in whole percentages.This will keep color/status behavior aligned between
getResourceColor()and the progress-bar columns.Also applies to: 58-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Filament/App/Resources/Servers/Pages/ListServers.php(5 hunks)app/Models/Node.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/App/Resources/Servers/Pages/ListServers.php (4)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (4)
ProgressBarColumn(10-265)warningThresholdPercent(61-66)dangerThresholdPercent(73-78)maxValue(49-54)app/Filament/Components/Tables/Columns/ServerEntryColumn.php (2)
warningThresholdPercent(42-47)dangerThresholdPercent(54-59)app/Models/Server.php (3)
Server(134-525)formatResource(485-511)retrieveResources(475-483)app/Enums/ServerResourceType.php (1)
getResourceAmount(21-35)
🔇 Additional comments (3)
app/Models/Node.php (2)
317-335: LGTM! Good caching strategy for system information.The 360-second cache TTL appropriately balances freshness with reduced load on the daemon API. The exception handling provides graceful degradation with user-friendly error messages.
370-370: Good defensive improvement.Changing from
$data['memory_total']to!empty($data['memory_total'])better handles edge cases where the key exists but contains null or zero values.app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
76-76: Uptime tooltip change looks good.Using
formatResource(ServerResourceType::Uptime, 2)keeps the tooltip formatting centralized and consistent with other resource displays; no issues spotted here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
174-214: Remove the unused getResourceColor method.The
getResourceColormethod is dead code—it's defined at lines 174-214 but never called anywhere in the codebase. Since its threshold logic is now handled byProgressBarColumn, this method can be safely removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/App/Resources/Servers/Pages/ListServers.php(5 hunks)
🔇 Additional comments (5)
app/Filament/App/Resources/Servers/Pages/ListServers.php (5)
8-8: LGTM!The import of
ProgressBarColumnand the threshold constants are well-defined and used consistently throughout the file.Also applies to: 38-40
58-59: LGTM!Properly configures the threshold percentages using the defined constants.
76-76: The method signature properly supports the varying parameter usage—no fixes needed.The method definition in app/Models/Server.php (line 485) declares
int $precision = 2as an optional parameter:public function formatResource(ServerResourceType $resourceType, int $precision = 2): stringThe varying usage across the codebase is intentional:
- CPU/CPULimit: precision 0 (percentage-based, no decimals)
- Memory/MemoryLimit/Disk/DiskLimit/Uptime: default precision 2 (absolute values with 2 decimal places)
This is a well-designed pattern where resource type determines precision requirements. No code changes are required.
Likely an incorrect or invalid review comment.
97-103: The original review comment is incorrect; no changes are needed.The
statistics()method inapp/Models/Node.php(line 346) always returns an array with guaranteed keys including'memory_total' => 0as a default value. Accessing$server->node->statistics()['memory_total']is safe and will never produce an undefined array key error. This is consistent with how the rest of the codebase accesses these keys directly without defensive null coalescing (e.g.,NodeMemoryChart.php:30,NodeStorageChart.php:40-75).Likely an incorrect or invalid review comment.
104-110: ****The review comment's concern is unfounded. The
statistics()method inapp/Models/Node.php(lines 346-383) guarantees that'disk_total'is always present in the returned array:
- It defines
$defaultwith'disk_total' => 0(line 357)- Return paths are either:
$datafrom the daemon API (if successful) or$default(if the request fails or lacks expected data)- Both paths ensure the key exists, eliminating any undefined array key risk
Direct array access without null coalescing is safe. The suggested defensive code change is unnecessary.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
5-22: Trait integration and default colors look solidUsing
HasProgresshere and wiring Filament theme colors insetUp()is consistent withProgressBarColumnand should keep server entries visually aligned. If you want stricter encapsulation, you could optionally switch to the trait setters (dangerColor(),warningColor(),color()) instead of touching the protected properties directly, but the current approach is functionally fine.app/Filament/Components/Tables/Columns/ProgressBarColumn.php (1)
21-30: ReuseHasProgresscolor resolver and align return typesThe progress math and thresholds look good, but
getProgressColor()is re‑implementing logic that already exists inHasProgress::getProgressColorForStatus(), and its declared return type omitsColoreven though the trait allowsColorinputs:
dangerColor(),warningColor(),color()inHasProgressacceptColorinstances.getProgressColorForStatus()already maps status → color and falls back to'gray'when all branches are null.To avoid divergence and better match the trait’s contract, you could:
- Delegate
getProgressColor()to the trait helper:public function getProgressColor(): string|array|Color { return $this->getProgressColorForStatus($this->getProgressStatus()); }
- Update the return type / phpdoc to include
Color(or, alternatively, normalize anyColorto a string/array here).This keeps all status→color rules in one place and makes the PHP types accurately reflect what callers — including your Blade view — may receive.
Also applies to: 61-71, 73-92, 107-135
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (1)
15-57: Clarify threshold semantics and centralize status→color mappingThe trait cleanly encapsulates thresholds and colors, and
getProgressColorForStatus()provides a nice single point for status→color resolution with sensible fallbacks.Two small improvements to consider:
- Document expected threshold range: Callers currently need to infer that
warningThresholdPercent/dangerThresholdPercentare fractions (e.g.0.7meaning 70%) rather than “percent points” (e.g.70). A short note in the method phpdocs (or property docblocks) stating “value between 0 and 1” would prevent misconfiguration.- Encourage reuse from columns:
ProgressBarColumn::getProgressColor()currently duplicates the same match/fallback logic; having it delegate togetProgressColorForStatus($this->getProgressStatus())will keep behavior consistent if you ever tweak the mapping or defaults here.These are non‑blocking, but they’ll make the trait easier to use correctly and reduce duplication in the columns.
Also applies to: 112-135
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php(1 hunks)app/Filament/Components/Tables/Columns/ProgressBarColumn.php(1 hunks)app/Filament/Components/Tables/Columns/ServerEntryColumn.php(1 hunks)resources/views/livewire/columns/progress-bar-column.blade.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (2)
app/Filament/Components/Tables/Columns/ServerEntryColumn.php (1)
setUp(15-22)app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (7)
dangerColor(61-66)getColor(107-110)warningColor(79-84)color(97-102)getDangerThresholdPercent(53-56)getDangerColor(71-74)getWarningColor(89-92)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
resources/views/livewire/server-entry.blade.php (2)
92-92: Add null coalescing formemory_totalto prevent undefined key errors.The
memory_totalkey is accessed without null coalescing. If the node statistics return incomplete data (e.g., on connection error), this will throw an undefined array key error.Apply this fix:
-$memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['memory_total'] : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server); +$memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['memory_total'] ?? 0) : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server);
114-114: Add null coalescing fordisk_totalto prevent undefined key errors.Same issue as memory—
disk_totalshould have a null coalescing fallback.Apply this fix:
-$diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['disk_total'] : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server); +$diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['disk_total'] ?? 0) : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server);app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
94-94: Critical operator precedence bug in CPU maxValue.Same issue as in the Blade file:
['cpu_count'] ?? 0 * 100evaluates as['cpu_count'] ?? 0due to operator precedence.Apply this fix:
-->maxValue(fn (Server $server) => ServerResourceType::CPULimit->getResourceAmount($server) === 0 ? ($server->node->systemInformation()['cpu_count'] ?? 0 * 100) : ServerResourceType::CPULimit->getResourceAmount($server)) +->maxValue(fn (Server $server) => ServerResourceType::CPULimit->getResourceAmount($server) === 0 ? (($server->node->systemInformation()['cpu_count'] ?? 0) * 100) : ServerResourceType::CPULimit->getResourceAmount($server))
🧹 Nitpick comments (1)
resources/views/livewire/columns/progress-bar-column.blade.php (1)
33-50: Minor: Extra whitespace in CSS selectors.There's extra whitespace before the
{in the keyframe and class declarations (Lines 35, 47). While most browsers handle this, it's cleaner to remove it.-@keyframes {{ $animClass }} { +@keyframes {{ $animClass }} {-.{{ $animClass }} { +.{{ $animClass }} {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/App/Resources/Servers/Pages/ListServers.php(5 hunks)app/Filament/Components/Tables/Columns/ProgressBarColumn.php(1 hunks)resources/views/livewire/columns/progress-bar-column.blade.php(1 hunks)resources/views/livewire/server-entry.blade.php(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-23T23:36:27.096Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1924
File: resources/views/livewire/server-entry.blade.php:6-7
Timestamp: 2025-11-23T23:36:27.096Z
Learning: In the Pelican Panel codebase, the `$server->node` relationship will never be null, so defensive null checks using the null-safe operator (`?->`) are not needed when accessing the node or its methods.
Applied to files:
resources/views/livewire/server-entry.blade.phpapp/Filament/App/Resources/Servers/Pages/ListServers.php
🧬 Code graph analysis (1)
app/Filament/Components/Tables/Columns/ProgressBarColumn.php (1)
app/Filament/Components/Tables/Columns/Concerns/HasProgress.php (8)
dangerColor(61-66)getColor(107-110)warningColor(79-84)color(97-102)getDangerThresholdPercent(53-56)getWarningThresholdPercent(41-44)getDangerColor(71-74)getWarningColor(89-92)
🔇 Additional comments (4)
app/Filament/App/Resources/Servers/Pages/ListServers.php (1)
38-40: Good addition of the WARNING_THRESHOLD constant.The threshold constants are well-defined and consistently applied across both grid and table column configurations.
resources/views/livewire/columns/progress-bar-column.blade.php (1)
53-67: Good accessibility implementation.The progress bar correctly implements ARIA attributes (
role="progressbar",aria-valuenow,aria-valuemin,aria-valuemax,aria-label) for screen reader support.app/Filament/Components/Tables/Columns/ProgressBarColumn.php (2)
60-70: Well-implemented percentage calculation with proper bounds.The method correctly handles null states and zero/negative max values, and clamps the result to 0-100%.
131-156: Solid color resolution logic.The static
resolveColormethod properly handles objects (with various color conversion methods), arrays (using shade 500 or first value), and strings. The final string check ensures type safety.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
resources/views/livewire/server-entry.blade.php (3)
91-92: Add null coalescing formemory_totalto match CPU pattern.Unlike the CPU block (line 70) which uses
?? 0forcpu_count, this line directly accesses$serverNodeStatistics['memory_total']. If the key is absent, PHP will emit a notice.- $memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['memory_total'] : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server); + $memMax = \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['memory_total'] ?? 0) : \App\Enums\ServerResourceType::MemoryLimit->getResourceAmount($server);
113-114: Add null coalescing fordisk_totalfor consistency.Same issue as the memory block—direct array access without null coalescing.
- $diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? $serverNodeStatistics['disk_total'] : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server); + $diskMax = \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server) === 0 ? ($serverNodeStatistics['disk_total'] ?? 0) : \App\Enums\ServerResourceType::DiskLimit->getResourceAmount($server);
67-131: Consider extracting repeated progress bar logic.The CPU, Memory, and Disk blocks share nearly identical structure. While acceptable for a Blade template, you could reduce duplication by iterating over a configuration array:
@php $resources = [ ['type' => \App\Enums\ServerResourceType::CPU, 'limitType' => \App\Enums\ServerResourceType::CPULimit, 'maxFallback' => ($serverNodeSystemInfo['cpu_count'] ?? 0) * 100, 'precision' => 0], ['type' => \App\Enums\ServerResourceType::Memory, 'limitType' => \App\Enums\ServerResourceType::MemoryLimit, 'maxFallback' => $serverNodeStatistics['memory_total'] ?? 0, 'precision' => null], ['type' => \App\Enums\ServerResourceType::Disk, 'limitType' => \App\Enums\ServerResourceType::DiskLimit, 'maxFallback' => $serverNodeStatistics['disk_total'] ?? 0, 'precision' => null], ]; @endphp @foreach($resources as $resource) <div class="w-full max-w-xs"> @php $current = $resource['type']->getResourceAmount($server); $max = $resource['limitType']->getResourceAmount($server) === 0 ? $resource['maxFallback'] : $resource['limitType']->getResourceAmount($server); // ... rest of closures @endphp @include('livewire.columns.progress-bar-column', [...]) </div> @endforeachThis is optional—the current explicit approach is also maintainable and clear.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
resources/views/livewire/server-entry.blade.php(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-23T23:36:27.096Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1924
File: resources/views/livewire/server-entry.blade.php:6-7
Timestamp: 2025-11-23T23:36:27.096Z
Learning: In the Pelican Panel codebase, the `$server->node` relationship will never be null, so defensive null checks using the null-safe operator (`?->`) are not needed when accessing the node or its methods.
Applied to files:
resources/views/livewire/server-entry.blade.php
🔇 Additional comments (2)
resources/views/livewire/server-entry.blade.php (2)
1-11: Initialization block looks good.The setup correctly initializes the server entry column with a sensible fallback, retrieves node statistics, and establishes threshold defaults (70% warning, 90% danger). Based on learnings, the
$server->noderelationship is guaranteed to exist, so no defensive null checks are needed here.
67-87: CPU progress bar implementation is correct.The operator precedence issue from the previous review has been properly fixed with
(($serverNodeSystemInfo['cpu_count'] ?? 0) * 100). The fallback logic, percentage calculation, and status determination are all sound.
Uh oh!
There was an error while loading. Please reload this page.