[Fix] Server provider availability#1158
Conversation
📝 WalkthroughWalkthroughControllers now return provider()->availablePlans(region) and providers produce structured plan entries ({label, available}). AbstractProvider filters available-only labels for API responses. Frontend normalises mixed plan shapes, disables unavailable options, and socket events are emitted for provider create/edit/delete. Tests and OpenAPI docs updated. ChangesServer Provider Plans API with Availability
Sequence Diagram(s) sequenceDiagram
participant Client
participant ServerProviderController
participant ServerProvider
participant AbstractProvider
Client->>ServerProviderController: GET /plans (region)
ServerProviderController->>ServerProvider: provider()->availablePlans(region)
ServerProvider->>AbstractProvider: availablePlans(region)
AbstractProvider->>ServerProvider: filters plans(), returns flat labels
ServerProvider-->>ServerProviderController: available plans map
ServerProviderController-->>Client: 200 JSON (plan-id => label)
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates server-provider plan handling to surface per-region availability (and pricing labels) in the web UI, while changing the API plans endpoints to return a flat, available-only plan map.
Changes:
- Provider
plans()implementations (Hetzner/DigitalOcean/Vultr/Linode) now computeavailableper plan+region and format labels with monthly pricing only when available. - Frontend server creation UI now supports both legacy string plan labels and
{ label, available }plan objects, disabling unavailable options. - API plans endpoints now return
availablePlans()(flatplanKey => labelmap) instead of provider-specific structures.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/ServerProvidersTest.php | Adds feature tests asserting plan availability/label formatting and ordering for multiple providers. |
| tests/Feature/API/ServerProvidersTest.php | Adds an API test asserting Hetzner plans are returned as a flat, available-only map. |
| resources/js/pages/servers/components/create-server.tsx | Normalizes plan data (string vs object) and disables unavailable plans in the selector UI. |
| app/ServerProviders/AbstractProvider.php | Introduces availablePlans() helper to flatten and filter plans for API responses. |
| app/ServerProviders/ServerProvider.php | Extends the provider contract with availablePlans(). |
| app/ServerProviders/Hetzner.php | Computes availability using location flags/deprecation and appends price when available. |
| app/ServerProviders/DigitalOcean.php | Returns all sizes with available per region and adds price only for available sizes. |
| app/ServerProviders/Vultr.php | Returns all plans with availability by region and adds price only for available plans. |
| app/ServerProviders/Linode.php | Computes availability by region capabilities and resolves regional pricing where present. |
| app/Http/Controllers/API/UserServerProviderController.php | Switches user-scoped API plans response to availablePlans(). |
| app/Http/Controllers/API/ServerProviderController.php | Switches project-scoped (deprecated) API plans response to availablePlans(). |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/pages/servers/components/create-server.tsx (1)
212-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
formin the useEffect dependency array to avoid stale closures.The
useEffectreferencesform.setDatain the event handlers but does not includeformin the dependency array. This can lead to stale closure bugs if theformreference changes.🔧 Proposed fix
useEffect(() => { if (defaultOpen) { setOpen(defaultOpen); } const handleRemoveService = (d: unknown) => { const service = d as Service; form.setData((data) => ({ ...data, services: data.services.filter((s) => s.type !== service.type || s.name !== service.name || s.version !== service.version), })); }; EventBus.on('remove-service', handleRemoveService); const handleAddService = (d: unknown) => { const service = d as Service; form.setData((data) => ({ ...data, services: [...data.services, service], })); }; EventBus.on('add-service', handleAddService); return () => { EventBus.off('remove-service', handleRemoveService); EventBus.off('add-service', handleAddService); }; - }, [defaultOpen]); + }, [defaultOpen, form]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/pages/servers/components/create-server.tsx` around lines 212 - 239, The effect registers EventBus handlers that call form.setData (handleRemoveService and handleAddService) but only lists defaultOpen in the dependency array, which risks stale closures; update the useEffect dependencies to include form so the handlers close over the current form instance (i.e., add form to the dependency array of the useEffect that defines handleRemoveService/handleAddService and registers them with EventBus) so the cleanup/off calls also remove the correct handlers.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/ServerProviders/DigitalOcean.php`:
- Around line 86-88: The PHPDoc for the DigitalOcean sizes response and the
mapping in plans() should treat the size['available'] field as a required
boolean (not optional); update the PHPDoc shape to declare "available: bool" and
in the ->map(function (array $size) use ($region): array { ... }) closure stop
using the fallback ($size['available'] ?? false) — either access
$size['available'] directly or cast it (bool) $size['available'] so the code and
types align with the API contract.
In `@app/ServerProviders/Linode.php`:
- Around line 127-138: The current planIsAvailable method silently treats empty
region capabilities as "available" (fail-open); change it to fail-closed by
returning false when $capabilities === [] and add a warning log when this
fallback happens so monitoring can detect capability-fetch failures; update the
planIsAvailable(string $class, array $capabilities) function to: if
$capabilities === [] then call the class's logger (e.g. $this->logger->warning
or the project's standard logger) with a clear message about missing/failed
region capabilities and return false, otherwise keep the existing match-based
checks for 'gpu', 'premium', and default.
---
Outside diff comments:
In `@resources/js/pages/servers/components/create-server.tsx`:
- Around line 212-239: The effect registers EventBus handlers that call
form.setData (handleRemoveService and handleAddService) but only lists
defaultOpen in the dependency array, which risks stale closures; update the
useEffect dependencies to include form so the handlers close over the current
form instance (i.e., add form to the dependency array of the useEffect that
defines handleRemoveService/handleAddService and registers them with EventBus)
so the cleanup/off calls also remove the correct handlers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 77680207-f698-46d3-868f-27a031c7af44
📒 Files selected for processing (11)
app/Http/Controllers/API/ServerProviderController.phpapp/Http/Controllers/API/UserServerProviderController.phpapp/ServerProviders/AbstractProvider.phpapp/ServerProviders/DigitalOcean.phpapp/ServerProviders/Hetzner.phpapp/ServerProviders/Linode.phpapp/ServerProviders/ServerProvider.phpapp/ServerProviders/Vultr.phpresources/js/pages/servers/components/create-server.tsxtests/Feature/API/ServerProvidersTest.phptests/Feature/ServerProvidersTest.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/js/pages/servers/components/create-server.tsx (1)
313-328:⚠️ Potential issue | 🟡 MinorStabilise
fetchServerProvidersfor hook dependency correctness.
fetchServerProvidersis recreated each render, but theuseEffectthat refreshes providers whenopenis true omits it from the dependency array. Memoise it withuseCallbackand include it in theuseEffectdeps (socket handling is safe here becauseuseSocketListenerkeeps the latest callback via a ref).Proposed fix
-import React, { FormEventHandler, useEffect, useState } from 'react'; +import React, { FormEventHandler, useCallback, useEffect, useState } from 'react'; - const fetchServerProviders = async () => { + const fetchServerProviders = useCallback(async () => { const serverProviders = await axios.get(route('server-providers.json')); setServerProviders(serverProviders.data); - }; + }, []); useEffect(() => { if (open) { - fetchServerProviders(); + void fetchServerProviders(); } - }, [open]); + }, [open, fetchServerProviders]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/pages/servers/components/create-server.tsx` around lines 313 - 328, fetchServerProviders is re-created on every render but is referenced by the useEffect that runs when open changes; memoise it with React.useCallback (e.g. const fetchServerProviders = useCallback(async () => { const res = await axios.get(route('server-providers.json')); setServerProviders(res.data); }, [])) and then include fetchServerProviders in the useEffect dependency array (useEffect(() => { if (open) fetchServerProviders(); }, [open, fetchServerProviders])); keep the existing useSocketListener usage as-is since it safely captures the latest callback via a ref.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@resources/js/pages/servers/components/create-server.tsx`:
- Around line 313-328: fetchServerProviders is re-created on every render but is
referenced by the useEffect that runs when open changes; memoise it with
React.useCallback (e.g. const fetchServerProviders = useCallback(async () => {
const res = await axios.get(route('server-providers.json'));
setServerProviders(res.data); }, [])) and then include fetchServerProviders in
the useEffect dependency array (useEffect(() => { if (open)
fetchServerProviders(); }, [open, fetchServerProviders])); keep the existing
useSocketListener usage as-is since it safely captures the latest callback via a
ref.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 45719168-b451-4312-b8bc-6f35c531f400
📒 Files selected for processing (8)
app/Actions/ServerProvider/CreateServerProvider.phpapp/Actions/ServerProvider/DeleteServerProvider.phpapp/Actions/ServerProvider/EditServerProvider.phpapp/ServerProviders/DigitalOcean.phppublic/api-docs/openapi/server-providers.yamlpublic/api-docs/openapi/user-server-providers.yamlresources/js/pages/servers/components/create-server.tsxtests/Feature/ServerProvidersTest.php
Summary by CodeRabbit
New Features
Tests
Documentation