Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Filament\Tables\Columns\IconColumn;
use Filament\Tables\Columns\TextColumn;
use Filament\Tables\Columns\TextInputColumn;
use Filament\Tables\Columns\ToggleColumn;
use Filament\Tables\Table;

/**
Expand Down Expand Up @@ -51,6 +52,8 @@ public function table(Table $table): Table
->label(trans('admin/server.port'))
->searchable()
->sortable(),
ToggleColumn::make('show_port')
->label(trans('admin/server.show_port')),
TextInputColumn::make('ip_alias')
->label(trans('admin/server.alias'))
->placeholder(trans('admin/server.no_alias'))
Expand Down Expand Up @@ -105,6 +108,7 @@ public function table(Table $table): Table
$allocation->update([
'notes' => null,
'is_locked' => false,
'show_port' => true,
]);

if (!$this->getOwnerRecord()->allocation_id) {
Expand All @@ -118,6 +122,7 @@ public function table(Table $table): Table
Allocation::whereNull('server_id')->update([
'notes' => null,
'is_locked' => false,
'show_port' => true,
]);

if (!$this->getOwnerRecord()->allocation_id) {
Expand Down
4 changes: 2 additions & 2 deletions app/Filament/App/Resources/Servers/Pages/ListServers.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ protected function tableColumns(): array
->grow()
->searchable()
->sortable(),
TextColumn::make('allocation.address')
TextColumn::make('allocation.display_address')
->label('')
->badge()
->visibleFrom('md')
->copyable()
->state(fn (Server $server) => $server->allocation->address ?? 'None'),
->state(fn (Server $server) => $server->allocation->display_address ?? 'None'),
Comment on lines +86 to +91

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the IP when no alias is set.

When show_port is false, Allocation::display_address returns alias. The alias is optional, so an allocation without an alias produces null. This column then displays None instead of the allocation IP without its port. The same accessor affects ServerOverview and both Livewire server entries.

Update Allocation::display_address to use the alias or the IP for the port-hidden case, and add a regression test.

Proposed fix
- get: fn () => $this->show_port ? $this->address : $this->alias,
+ get: fn () => $this->show_port ? $this->address : ($this->alias ?? $this->ip),
🤖 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 `@app/Filament/App/Resources/Servers/Pages/ListServers.php` around lines 86 -
91, Update Allocation::display_address so the show_port=false branch returns the
optional alias when present and otherwise falls back to the allocation IP
without its port. Add a regression test covering an allocation without an alias,
and verify the shared accessor behavior used by the server list, ServerOverview,
and Livewire server entries.

ProgressBarColumn::make('cpuUsage')
->label('')
->warningThresholdPercent(static::WARNING_THRESHOLD)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ public static function defaultTable(Table $table): Table
TextColumn::make('alias')
->hidden(),
TextColumn::make('port')
->label(trans('server/network.port')),
->label(trans('server/network.port'))
->state(fn (Allocation $allocation) => $allocation->show_port ? $allocation->port : null)
->placeholder('—'),
TextInputColumn::make('notes')
->label(trans('server/network.notes'))
->visibleFrom('sm')
Expand Down Expand Up @@ -88,12 +90,13 @@ public static function defaultTable(Table $table): Table
Allocation::where('id', $allocation->id)->update([
'notes' => null,
'is_locked' => false,
'show_port' => true,
'server_id' => null,
]);

Activity::event('server:allocation.delete')
->subject($allocation)
->property('allocation', $allocation->address)
->property('allocation', $allocation->display_address)
->log();
})
->after(fn (Allocation $allocation) => $allocation->id === $server->allocation_id && $server->update(['allocation_id' => $server->allocations()->first()?->id])),
Expand All @@ -116,7 +119,7 @@ public static function defaultTable(Table $table): Table

Activity::event('server:allocation.create')
->subject($allocation)
->property('allocation', $allocation->address)
->property('allocation', $allocation->display_address)
->log();
}),
]);
Expand Down
2 changes: 1 addition & 1 deletion app/Filament/Server/Widgets/ServerOverview.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ protected function getStats(): array
SmallStatBlock::make(trans('server/console.labels.name'), $this->server->name)
->copyable(),
SmallStatBlock::make(trans('server/console.labels.status'), $this->status()),
SmallStatBlock::make(trans('server/console.labels.address'), $this->server?->allocation->address ?? 'None')
SmallStatBlock::make(trans('server/console.labels.address'), $this->server?->allocation->display_address ?? 'None')
->copyable(),
SmallStatBlock::make(trans('server/console.labels.cpu'), $this->cpuUsage()),
SmallStatBlock::make(trans('server/console.labels.memory'), $this->memoryUsage()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function update(UpdateAllocationRequest $request, Server $server, Allocat
if ($original !== $allocation->notes) {
Activity::event('server:allocation.notes')
->subject($allocation)
->property(['allocation' => $allocation->address, 'old' => $original, 'new' => $allocation->notes])
->property(['allocation' => $allocation->display_address, 'old' => $original, 'new' => $allocation->notes])
->log();
}

Expand All @@ -88,7 +88,7 @@ public function setPrimary(SetPrimaryAllocationRequest $request, Server $server,

Activity::event('server:allocation.primary')
->subject($allocation)
->property('allocation', $allocation->address)
->property('allocation', $allocation->display_address)
->log();

return $this->fractal->item($allocation)
Expand All @@ -114,7 +114,7 @@ public function store(NewAllocationRequest $request, Server $server): array

$allocation = $this->assignableAllocationService->handle($server);

$log->subject($allocation)->property('allocation', $allocation->address);
$log->subject($allocation)->property('allocation', $allocation->display_address);

return $allocation;
});
Expand All @@ -138,13 +138,15 @@ public function delete(DeleteAllocationRequest $request, Server $server, Allocat
throw_if(empty($server->allocation_limit), new DisplayException('You cannot delete allocations for this server: no allocation limit is set.'));

Allocation::query()->where('id', $allocation->id)->update([
'is_locked' => false,
'notes' => null,
'show_port' => true,
'server_id' => null,
]);

Activity::event('server:allocation.delete')
->subject($allocation)
->property('allocation', $allocation->address)
->property('allocation', $allocation->display_address)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
->log();

return new JsonResponse([], JsonResponse::HTTP_NO_CONTENT);
Expand Down
15 changes: 15 additions & 0 deletions app/Models/Allocation.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
* @property string|null $ip_alias
* @property string|null $notes
* @property bool $is_locked
* @property bool $show_port
* @property-read string $address
* @property-read string $alias
* @property-read string $display_address
* @property-read bool $has_alias
* @property-read Node $node
* @property-read Server|null $server
Expand All @@ -43,6 +45,7 @@
* @method static Builder<static>|Allocation whereNotes($value)
* @method static Builder<static>|Allocation wherePort($value)
* @method static Builder<static>|Allocation whereServerId($value)
* @method static Builder<static>|Allocation whereShowPort($value)
* @method static Builder<static>|Allocation whereUpdatedAt($value)
*/
class Allocation extends Model
Expand All @@ -58,6 +61,7 @@ class Allocation extends Model

protected $attributes = [
'is_locked' => false,
'show_port' => true,
];

/**
Expand All @@ -74,13 +78,15 @@ class Allocation extends Model
'server_id' => ['nullable', 'exists:servers,id'],
'notes' => ['nullable', 'string', 'max:256'],
'is_locked' => ['boolean'],
'show_port' => ['boolean'],
];

protected static function booted(): void
{
static::updating(function (self $allocation) {
if (is_null($allocation->server_id)) {
$allocation->is_locked = false;
$allocation->show_port = true;
}
});

Expand All @@ -96,6 +102,7 @@ protected function casts(): array
'port' => 'integer',
'server_id' => 'integer',
'is_locked' => 'bool',
'show_port' => 'bool',
];
}

Expand Down Expand Up @@ -123,6 +130,14 @@ protected function address(): Attribute
);
}

/** @return Attribute<string, never> */
protected function displayAddress(): Attribute
{
return Attribute::make(
get: fn () => $this->show_port ? $this->address : ($this->ip_alias ?? $this->ip),
);
}

/**
* Gets information for the server associated with this allocation.
*/
Expand Down
1 change: 1 addition & 0 deletions app/Transformers/Api/Application/AllocationTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public function transform($allocation): array
'ip' => $allocation->ip,
'alias' => $allocation->ip_alias,
'port' => $allocation->port,
'show_port' => $allocation->show_port,
'notes' => $allocation->notes,
'assigned' => !is_null($allocation->server_id),
];
Expand Down
1 change: 1 addition & 0 deletions app/Transformers/Api/Client/AllocationTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public function transform($model): array
'ip' => $model->ip,
'ip_alias' => $model->ip_alias,
'port' => $model->port,
'show_port' => $model->show_port,
'notes' => $model->notes,
'is_default' => $model->server->allocation_id === $model->id,
];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
public function up(): void
{
Schema::table('allocations', function (Blueprint $table) {
$table->boolean('show_port')->default(true)->after('port');
});
}

public function down(): void
{
Schema::table('allocations', function (Blueprint $table) {
$table->dropColumn('show_port');
});
}
};
1 change: 1 addition & 0 deletions lang/en/admin/server.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
'ip_address_helper' => 'Usually your machine\'s public IP unless you are port forwarding.',
'port' => 'Port',
'ports' => 'Ports',
'show_port' => 'Show port',
'alias' => 'Alias',
'alias_helper' => 'Optional display name to help you remember what these are.',
'no_alias' => 'No Alias',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@

<div class="hidden sm:block">
<p class="text-sm dark:text-gray-400">{{ trans('server/dashboard.network') }}</p>
<p class="text-md font-semibold">{{ $server->allocation?->address ?? trans('server/dashboard.none') }}</p>
<p class="text-md font-semibold">{{ $server->allocation?->display_address ?? trans('server/dashboard.none') }}</p>
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion resources/views/livewire/server-entry.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class="relative cursor-pointer"

<div class="hidden sm:block">
<p class="text-sm dark:text-gray-400">{{ trans('server/dashboard.network') }}</p>
<p class="text-md font-semibold">{{ $server->allocation?->address ?? trans('server/dashboard.none') }}</p>
<p class="text-md font-semibold">{{ $server->allocation?->display_address ?? trans('server/dashboard.none') }}</p>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,20 @@ public function test_allocation_can_be_deleted_from_server(array $permission): v
$allocation = Allocation::factory()->create([
'server_id' => $server->id,
'node_id' => $server->node_id,
'is_locked' => true,
'notes' => 'hodor',
'show_port' => false,
]);

$this->actingAs($user)->deleteJson($this->link($allocation))->assertStatus(Response::HTTP_NO_CONTENT);

$this->assertDatabaseHas('allocations', ['id' => $allocation->id, 'server_id' => null, 'notes' => null]);
$this->assertDatabaseHas('allocations', [
'id' => $allocation->id,
'is_locked' => false,
'server_id' => null,
'notes' => null,
'show_port' => true,
]);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ public function test_server_allocations_are_returned(): void
$this->assertJsonTransformedWith($response->json('data.0.attributes'), $server->allocation);
}

public function test_port_visibility_preference_is_returned(): void
{
[$user, $server] = $this->generateTestAccount();
$server->allocation->update(['show_port' => false]);

$this->actingAs($user)
->getJson($this->link($server, '/network/allocations'))
->assertOk()
->assertJsonPath('data.0.attributes.port', $server->allocation->port)
->assertJsonPath('data.0.attributes.show_port', false);
}

/**
* Test that allocations cannot be returned without the required user permissions.
*/
Expand Down
58 changes: 58 additions & 0 deletions tests/Unit/Models/AllocationTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace App\Tests\Unit\Models;

use App\Models\Allocation;
use App\Tests\TestCase;
use PHPUnit\Framework\Attributes\DataProvider;

class AllocationTest extends TestCase
{
#[DataProvider('displayAddressDataProvider')]
public function test_display_address_respects_port_visibility(string $ip, ?string $alias, bool $showPort, string $expected): void
{
$allocation = new Allocation([
'ip' => $ip,
'ip_alias' => $alias,
'port' => 25565,
'show_port' => $showPort,
]);

$this->assertSame($expected, $allocation->display_address);
}

public function test_port_is_shown_by_default(): void
{
$allocation = new Allocation([
'ip' => '192.0.2.1',
'port' => 25565,
]);

$this->assertTrue($allocation->show_port);
$this->assertSame('192.0.2.1:25565', $allocation->display_address);
}

public function test_hidden_port_falls_back_to_ip_when_alias_is_missing(): void
{
$allocation = new Allocation([
'ip' => '192.0.2.1',
'ip_alias' => null,
'port' => 25565,
'show_port' => false,
]);

$this->assertSame('192.0.2.1', $allocation->display_address);
}

public static function displayAddressDataProvider(): array
{
return [
'IPv4 with port' => ['192.0.2.1', null, true, '192.0.2.1:25565'],
'IPv4 without port' => ['192.0.2.1', null, false, '192.0.2.1'],
'IPv6 with port' => ['2001:db8::1', null, true, '[2001:db8::1]:25565'],
'IPv6 without port' => ['2001:db8::1', null, false, '2001:db8::1'],
'alias with port' => ['192.0.2.1', 'play.example.com', true, 'play.example.com:25565'],
'alias without port' => ['192.0.2.1', 'play.example.com', false, 'play.example.com'],
];
}
}
Loading