Create custom number format method to catch invalid languages on php 8.4 #1623
Conversation
📝 WalkthroughWalkthroughIntroduces a global format_number(...) helper (with user-language fallback) and replaces direct uses of Illuminate\Support\Number::format across widgets, pages, models, and helpers. Some chart widgets now emit numeric values via round(...) instead of formatted strings. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Widget/Page/Model/Helper
participant F as format_number()
participant N as Number::format()
Caller->>F: format_number(value, precision?, maxPrecision?)
F->>N: Number::format(value, precision?, maxPrecision?, auth()->user()->language ?? 'en')
alt Number::format throws
F->>N: Number::format(value, precision?, maxPrecision?, 'en')
end
N-->>F: formatted string
F-->>Caller: formatted string
sequenceDiagram
participant Widget as ServerCpu/ServerMemoryChart
participant R as round()
Widget->>R: round(value, 2)
R-->>Widget: numeric float (2 decimals)
Widget-->>Chart: dataset with numeric values
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
app/Filament/Admin/Resources/NodeResource/Widgets/NodeMemoryChart.php (1)
82-95: Fix latest value retrieval; array_slice on end(...) breaks ['memory'] accessLine 84 calls array_slice(end($this->memoryHistory), -60), which reindexes keys numerically. Accessing $latestMemoryUsed['memory'] will be undefined. Also, handle empty history safely, and avoid duplicating the binary/decimal branches.
Apply:
- $latestMemoryUsed = array_slice(end($this->memoryHistory), -60); - - $used = config('panel.use_binary_prefix') - ? format_number($latestMemoryUsed['memory'], maxPrecision: 2) .' GiB' - : format_number($latestMemoryUsed['memory'], maxPrecision: 2) . ' GB'; - - $total = config('panel.use_binary_prefix') - ? format_number($this->totalMemory / 1024 / 1024 / 1024, maxPrecision: 2) .' GiB' - : format_number($this->totalMemory / 1000 / 1000 / 1000, maxPrecision: 2) . ' GB'; + $latestMemoryUsed = end($this->memoryHistory) ?: ['memory' => 0.0, 'timestamp' => now(auth()->user()->timezone ?? 'UTC')->format('H:i:s')]; + $useBinary = config('panel.use_binary_prefix'); + $usedValue = (float) $latestMemoryUsed['memory']; + $totalValue = $useBinary + ? $this->totalMemory / 1024 / 1024 / 1024 + : $this->totalMemory / 1000 / 1000 / 1000; + + $unitLabel = $useBinary ? ' GiB' : ' GB'; + $used = format_number($usedValue, maxPrecision: 2) . $unitLabel; + $total = format_number($totalValue, maxPrecision: 2) . $unitLabel;Additionally, the docblock at Line 18 states memory: string, but memory is now numeric. Consider updating it to memory: float.
🧹 Nitpick comments (5)
app/Filament/Server/Widgets/ServerOverview.php (1)
56-56: Consistent formatting; consider no decimals for limits
- Line 56: Using format_number with maxPrecision: 2 for live CPU is appropriate.
- Line 58: For the configured CPU limit (an integer in most setups), consider precision: 0 to avoid rendering a trailing “.00 %”.
Proposed change for Line 58 only:
- return $cpu . ($this->server->cpu > 0 ? ' / ' . format_number($this->server->cpu) . ' %' : ' / ∞'); + return $cpu . ($this->server->cpu > 0 ? ' / ' . format_number($this->server->cpu, precision: 0) . ' %' : ' / ∞');Also applies to: 58-58
app/Filament/Admin/Resources/NodeResource/Widgets/NodeCpuChart.php (1)
84-86: Update docblock to reflect numeric cpu; optional: avoid decimals for max
- Given getData() uses round(..., 2) for 'cpu', $this->cpuHistory now holds numeric floats, but the docblock at Line 18 still declares cpu: string. Update it to cpu: float for accuracy.
- For $max (threads * 100), consider precision: 0 since this should be a whole number.
Outside this hunk, update the docblock:
/** * @var array<int, array{cpu: float, timestamp: string}> */And adjust Line 85 if you want whole numbers:
- $max = format_number($this->threads * 100); + $max = format_number($this->threads * 100, precision: 0);app/Filament/Server/Widgets/ServerMemoryChart.php (1)
33-35: Micro-optimization: avoid repeated config lookups inside the mapconfig('panel.use_binary_prefix') is evaluated for every element. Cache it in a local $useBinary (and optionally a $unit = $useBinary ? 1024 : 1000) outside the map for readability and tiny perf wins.
app/Models/Server.php (2)
486-487: Guard against non-string return from format_number when concatenating '%'format_number is declared as false|string. If it ever returns false, concatenation produces just '%' which is misleading. Prefer ensuring a string, or make format_number always return string (see helpers.php comment).
For a local guard here:
- return format_number($resourceAmount, precision: 2) . '%'; + $formatted = (string) format_number($resourceAmount, precision: 2); + return $formatted . '%';
480-484: Use logical AND (&&) instead of bitwise &The condition uses bitwise AND, which happens to work via type juggling but is misleading and error-prone.
- if ($resourceAmount === 0 & $resourceType->isLimit()) { + if ($resourceAmount === 0 && $resourceType->isLimit()) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/Filament/Admin/Resources/NodeResource/Widgets/NodeCpuChart.php(1 hunks)app/Filament/Admin/Resources/NodeResource/Widgets/NodeMemoryChart.php(1 hunks)app/Filament/Server/Pages/Settings.php(1 hunks)app/Filament/Server/Widgets/ServerCpuChart.php(1 hunks)app/Filament/Server/Widgets/ServerMemoryChart.php(1 hunks)app/Filament/Server/Widgets/ServerOverview.php(1 hunks)app/Livewire/ServerEntry.php(1 hunks)app/Models/Server.php(1 hunks)app/helpers.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/Models/Server.php (1)
app/helpers.php (1)
format_number(103-111)
app/Filament/Admin/Resources/NodeResource/Widgets/NodeMemoryChart.php (1)
app/helpers.php (1)
format_number(103-111)
app/Filament/Server/Widgets/ServerOverview.php (1)
app/helpers.php (1)
format_number(103-111)
app/Filament/Server/Pages/Settings.php (2)
app/Models/Server.php (1)
Server(126-498)app/helpers.php (1)
format_number(103-111)
app/Filament/Admin/Resources/NodeResource/Widgets/NodeCpuChart.php (1)
app/helpers.php (1)
format_number(103-111)
🔇 Additional comments (4)
app/Filament/Server/Widgets/ServerCpuChart.php (1)
33-35: Good move: feed Chart.js numeric datapointsSwitching to round($value, 2) ensures the dataset is numeric, avoiding locale-induced commas in strings that can break chart rendering. This is the right direction for chart data.
app/Livewire/ServerEntry.php (1)
41-41: LGTM: consistent with centralized formattingUsing format_number here keeps the placeholder consistent with the rest of the UI and avoids locale-specific pitfalls.
app/Filament/Server/Widgets/ServerMemoryChart.php (1)
33-34: Good change: emit numeric values to the chartSwitching from localized strings to round(..., 2) yields proper numeric data for Chart.js and avoids locale parsing issues. Keep it.
app/helpers.php (1)
48-49: LGTM: centralize formatting in convert_bytes_to_readableDelegating to format_number is the right direction to keep locale handling consistent.
#SavePirateLanguage