Skip to content

refactor: site and server CRUD improvements #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
loadinglucian merged 3 commits into from
Nov 1, 2025
+174 −152
Merged

refactor: site and server CRUD improvements #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97b52bd
refactor: site and server CRUD improvements
loadinglucian Oct 31, 2025
c998fb0
fix: misleading warning message when server not found while adding site
loadinglucian Nov 1, 2025
595bff4
fix: rector issue
loadinglucian Nov 1, 2025
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
1 change: 1 addition & 0 deletions app/Console/Server/ServerDeleteCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->io->hr();

$this->displayServerDeets($server, $serverSites);
$this->io->writeln('');

if (count($serverSites) > 0) {
$this->io->error("Cannot delete server '{$server->name}' because it has one or more sites.");
Expand Down
62 changes: 17 additions & 45 deletions app/Console/Site/SiteAddCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Bigpixelrocket\DeployerPHP\Contracts\BaseCommand;
use Bigpixelrocket\DeployerPHP\DTOs\SiteDTO;
use Bigpixelrocket\DeployerPHP\Traits\ServerHelpersTrait;
use Bigpixelrocket\DeployerPHP\Traits\SiteHelpersTrait;
use Bigpixelrocket\DeployerPHP\Traits\SiteValidationTrait;
use Symfony\Component\Console\Attribute\AsCommand;
Expand All @@ -22,6 +23,7 @@
#[AsCommand(name: 'site:add', description: 'Add a new site to the inventory')]
class SiteAddCommand extends BaseCommand
{
use ServerHelpersTrait;
use SiteHelpersTrait;
use SiteValidationTrait;

Expand All @@ -35,10 +37,10 @@ protected function configure(): void

$this
->addOption('domain', null, InputOption::VALUE_REQUIRED, 'Domain name')
->addOption('type', null, InputOption::VALUE_REQUIRED, 'Site type: git or local')
->addOption('source', null, InputOption::VALUE_REQUIRED, 'Site source: git or local')
->addOption('repo', null, InputOption::VALUE_REQUIRED, 'Git repository URL (for git sites)')
->addOption('branch', null, InputOption::VALUE_REQUIRED, 'Git branch name (for git sites)')
->addOption('servers', null, InputOption::VALUE_REQUIRED, 'Comma-separated server names');
->addOption('server', null, InputOption::VALUE_REQUIRED, 'Server name');
}
Comment thread
loadinglucian marked this conversation as resolved.

//
Expand All @@ -50,23 +52,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
parent::execute($input, $output);

$this->io->hr();

$this->io->h1('Add New Site');

//
// Check if there are any servers

if (count($this->servers->all()) === 0) {
$this->io->warning('No servers available');
$this->io->writeln([
'',
'You must add at least one server before adding a site.',
'Run <fg=cyan>server:provision</> to provision your first server,',
'or run <fg=cyan>server:add</> to add an existing server.',
'',
]);
// Select server

return Command::FAILURE;
$server = $this->selectServer();

if (is_int($server)) {
return $server;
}
Comment on lines +60 to 64

@coderabbitai coderabbitai Bot Nov 1, 2025

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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify selectServer() return values for proper error handling.

The current code correctly propagates the integer status from selectServer(). However, ensure that selectServer() returns Command::FAILURE (not Command::SUCCESS) when no servers are available, so the command doesn't exit successfully without adding a site.

🏁 Script executed:

#!/bin/bash
# Check selectServer() implementation to verify return values
ast-grep --pattern $'function selectServer($$$) {
  $$$
}'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find ServerHelpersTrait file
fd -t f "ServerHelpersTrait" --type f

Length of output: 33

🏁 Script executed:

#!/bin/bash
# Search for selectServer function definition
rg -n "function selectServer" --type php -A 20

Length of output: 1437

🏁 Script executed:

#!/bin/bash
# Also search for selectServer calls to understand usage
rg -n "selectServer" --type php -B 2 -A 2

Length of output: 1092

🏁 Script executed:

#!/bin/bash
# Find ensureServersAvailable() implementation
rg -n "function ensureServersAvailable" --type php -A 15

Length of output: 1157

🏁 Script executed:

#!/bin/bash
# Look for any warnings or messages about "no servers" or "must add"
rg -n "must add\|no servers" --type php -i -B 2 -A 2

Length of output: 0

Fix ensureServersAvailable() to return Command::FAILURE when no servers exist.

The verification confirms a critical issue: ensureServersAvailable() in app/Traits/ServerHelpersTrait.php:43 returns Command::SUCCESS when no servers are available. This causes the command to exit with status 0 (success) despite failing to add a site. The method should return Command::FAILURE instead. Update line 43 from return Command::SUCCESS; to return Command::FAILURE; and correct the docblock on line 52.

🤖 Prompt for AI Agents 
In app/Traits/ServerHelpersTrait.php around line 43 (and docblock at line 52),
change the early return when no servers are available from Command::SUCCESS to
Command::FAILURE so the calling command exits with a failure status; also update
the docblock on line 52 to reflect that the method returns int
(Command::SUCCESS|Command::FAILURE) or clearly state it returns a Command exit
code instead of implying success.


//
Expand All @@ -89,19 +83,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

//
// Select site type
// Select site source

/** @var string $siteType */
$siteType = $this->io->getOptionOrPrompt(
'type',
/** @var string $siteSource */
$siteSource = $this->io->getOptionOrPrompt(
'source',
fn (): string => (string) $this->io->promptSelect(
label: 'Deploy from:',
options: ['git' => 'Git Repository', 'local' => 'Local files'],
default: 'git'
)
);

$isLocal = $siteType === 'local';
$isLocal = $siteSource === 'local';

//
// Gather git-specific details
Expand Down Expand Up @@ -149,36 +143,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
}

//
// Select servers

try {
$selectedServers = $this->selectServers();
} catch (\RuntimeException $e) {
$this->io->error($e->getMessage());

return Command::FAILURE;
}

//
// Validate selections

try {
$this->validateServers($selectedServers);
} catch (\RuntimeException $e) {
$this->io->error($e->getMessage());

return Command::FAILURE;
}

//
// Create DTO and display site info

$site = new SiteDTO(
domain: $domain,
repo: $repo,
branch: $branch,
servers: $selectedServers
servers: [$server->name]

@coderabbitai coderabbitai Bot Nov 1, 2025

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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Clarify single-server design decision.

The change from multi-server to single-server selection is significant. While this may simplify the UX, users who previously added a site to multiple servers in one command will now need multiple invocations.

Is this intentional? Will users be able to add the same site to multiple servers easily, or is this a breaking change that should be documented?

🌐 Web query:

Check if there's a follow-up command or feature planned for multi-server site management in deployer-php

💡 Result:

Do you mean the Deployer (deployer.org / deployerphp) project or the “Deployer for Git” WordPress plugin — or another "deployer-php" repo? If you tell me which repository or provide a link, I’ll search the project issues/roadmap and report any planned follow-up command/feature for multi‑server site management (with citations).

I need to search the repository more specifically. Let me explore the codebase to understand the design decision and look for any documentation or issues about multi-server support.


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for multi-server references and DTO definitions
rg -n "SiteDTO|servers" --type=php -A2 -B2 | head -50

Length of output: 3150

🏁 Script executed:

#!/bin/bash
# Check for any comments or TODOs about multi-server support
rg -n "TODO|FIXME|multi.*server|multi-server" --type=php | head -30

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Look for git history or comments explaining the change
git log -n 20 --oneline -- app/Console/Site/SiteAddCommand.php 2>/dev/null || echo "Git history not available"

Length of output: 169

🏁 Script executed:

#!/bin/bash
# Search for issues or feature discussions in common locations
find . -type f \( -name "*.md" -o -name "CHANGELOG*" -o -name "TODO*" -o -name ".github" \) -exec grep -l "multi.*server\|SiteAdd" {} \; 2>/dev/null | head -10

Length of output: 0


---

🏁 Script executed:

```shell
#!/bin/bash
# Find and inspect SiteDTO definition
fd "SiteDTO" --type=f | head -5

Length of output: 20

🏁 Script executed:

#!/bin/bash
# Get full SiteAddCommand.php to see complete context
cat app/Console/Site/SiteAddCommand.php | head -200

Length of output: 5569

🏁 Script executed:

#!/bin/bash
# Search for other site commands to see if multi-server is handled elsewhere
fd "Site.*Command.php" --type=f | xargs ls -la 2>/dev/null

Length of output: 258

🏁 Script executed:

#!/bin/bash
# Look for Site management services that might handle multi-server
rg -n "class.*Service.*Site\|function.*addSite\|function.*createSite" --type=php -A3 | head -40

Length of output: 0


---

🏁 Script executed:

```shell
#!/bin/bash
# Inspect SiteDTO structure
cat app/DTOs/SiteDTO.php

Length of output: 936

🏁 Script executed:

#!/bin/bash
# Find and inspect SiteRepository.create() method
rg -n "class SiteRepository\|function create\|public function create" --type=php -A15 app/Repositories/SiteRepository.php 2>/dev/null | head -60

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check SiteDeleteCommand to see if it handles multi-server deletes
cat app/Console/Site/SiteDeleteCommand.php

Length of output: 3215

🏁 Script executed:

#!/bin/bash
# Look at git diff for the refactor commit to understand what changed
git show 97b52bd --stat 2>/dev/null | head -30

Length of output: 733


---

🏁 Script executed:

```shell
#!/bin/bash
# Get the actual diff of SiteAddCommand from the refactor to see what changed
git show 97b52bd -- app/Console/Site/SiteAddCommand.php 2>/dev/null | head -150

Length of output: 4894

🏁 Script executed:

#!/bin/bash
# Search for any edit/update site command or mechanism
fd "SiteUpdateCommand\|SiteEditCommand\|updateSite\|editSite" --type=f

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for any way to modify existing sites (add servers, etc.)
rg -n "class.*Site.*\|update.*site\|edit.*site\|add.*server.*site" --type=php -i | grep -v "test\|Test" | head -20

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check if there are any comments in code about multi-server or server list
rg -n "servers|multiple.*server" app/Console/Site/ --type=php -B2 -A2 | head -40

Length of output: 302

Document the single-server limitation or provide a migration path.

The refactor intentionally changed --servers (plural, multi-select) to --server (singular), removing the ability to add a site to multiple servers in one command. While this simplifies the UX, it's a breaking change with no documented workaround.

Either document this limitation explicitly in help text or CHANGELOG, or add a site:update command to modify servers after creation.

);

$this->io->hr();
Expand All @@ -204,8 +176,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int

$hintOptions = [
'domain' => $domain,
'type' => $siteType,
'servers' => implode(',', $selectedServers),
'source' => $siteSource,
'server' => $server->name,
];

if (!$isLocal) {
Expand Down
36 changes: 27 additions & 9 deletions app/Console/Site/SiteDeleteCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ protected function configure(): void

$this
->addOption('site', null, InputOption::VALUE_REQUIRED, 'Site domain')
->addOption('force', null, InputOption::VALUE_NONE, 'Skip typing site domain (use with caution)')
->addOption('yes', 'y', InputOption::VALUE_NONE, 'Skip confirmation prompt');
}

Expand All @@ -42,32 +43,48 @@ protected function execute(InputInterface $input, OutputInterface $output): int
parent::execute($input, $output);

$this->io->hr();

$this->io->h1('Delete Site');

//
// Select site

$selection = $this->selectSite();
$site = $this->selectSite();

if ($selection['site'] === null) {
return $selection['exit_code'];
if (!$site instanceof \Bigpixelrocket\DeployerPHP\DTOs\SiteDTO) {
return $site;
}

$site = $selection['site'];
$this->io->hr();

$this->displaySiteDeets($site);
$this->io->writeln('');

//
// Confirm deletion
// Confirm deletion with extra safety

$this->io->writeln('');
/** @var bool $forceSkip */
$forceSkip = $input->getOption('force') ?? false;

if (!$forceSkip) {
$typedDomain = $this->io->promptText(
label: "Type the site domain '{$site->domain}' to confirm deletion:",
required: true
);

if ($typedDomain !== $site->domain) {
$this->io->error('Site domain does not match. Deletion cancelled.');
$this->io->writeln('');

return Command::FAILURE;
}
}

/** @var bool $confirmed */
$confirmed = $this->io->getOptionOrPrompt(
'yes',
fn (): bool => $this->io->promptConfirm(
label: 'Are you sure you want to delete this site?',
default: true
label: 'Are you absolutely sure?',
default: false
)
);

Expand All @@ -92,6 +109,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->io->showCommandHint('site:delete', [
'site' => $site->domain,
'yes' => $confirmed,
'force' => true,
]);
Comment thread
loadinglucian marked this conversation as resolved.

return Command::SUCCESS;
Expand Down
27 changes: 15 additions & 12 deletions app/Console/Site/SiteListCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
/**
* List all sites in the inventory.
*/
#[AsCommand(name: 'site:list', description: 'List all sites in the inventory')]
#[AsCommand(name: 'site:list', description: 'List sites in the inventory')]
class SiteListCommand extends BaseCommand
{
use SiteHelpersTrait;
Expand All @@ -28,26 +28,29 @@ protected function execute(InputInterface $input, OutputInterface $output): int
parent::execute($input, $output);

$this->io->hr();
$this->io->h1('List Sites');

//
// Get all sites

$allSites = $this->sites->all();
if (count($allSites) === 0) {
$this->io->warning('No sites found in inventory');
$this->io->writeln([
'',
'Use <fg=cyan>site:add</> to add a site',
'',
]);
$allSites = $this->ensureSitesAvailable();

return Command::SUCCESS;
if (is_int($allSites)) {
return $allSites;
}

$this->io->h1('All Sites');
//
// Display sites

foreach ($allSites as $site) {
foreach ($allSites as $count => $site) {
$this->displaySiteDeets($site);

if ($count < count($allSites) - 1) {
$this->io->writeln([
' ───',
'',
]);
}
}

return Command::SUCCESS;
Expand Down
43 changes: 43 additions & 0 deletions app/Services/IOService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,49 @@ public function hr(): void
]);
}

/**
* Display key-value details with aligned formatting.
*
* Formats key-value pairs with proper alignment and gray styling for values.
*
* @param array<string, string|int|float|bool|null|array<int, string>> $details Key-value pairs to display
*
* @example
* $this->io->displayDeets([
* 'Name' => 'production-web-01',
* 'Host' => '192.168.1.100',
* 'Port' => 22,
* ]);
* // Output:
* // Name: production-web-01
* // Host: 192.168.1.100
* // Port: 22
*/
public function displayDeets(array $details): void
{
if (empty($details)) {
return;
}

// Find longest key for alignment
$maxLength = max(array_map(strlen(...), array_keys($details)));

$lines = [];
foreach ($details as $key => $value) {
$paddedKey = str_pad($key.':', $maxLength + 1);
if (is_array($value)) {
$lines[] = " {$paddedKey}";
foreach ($value as $item) {
$lines[] = " <fg=gray>• {$item}</>";
}
} else {
$lines[] = " {$paddedKey} <fg=gray>{$value}</>";
}
}

$this->writeln($lines);
Comment thread
loadinglucian marked this conversation as resolved.
}

/**
* Display a command replay hint showing how to run non-interactively.
*
Expand Down
28 changes: 15 additions & 13 deletions app/Traits/ServerHelpersTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Bigpixelrocket\DeployerPHP\Traits;

use Bigpixelrocket\DeployerPHP\DTOs\ServerDTO;
use Bigpixelrocket\DeployerPHP\DTOs\SiteDTO;
use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository;
use Bigpixelrocket\DeployerPHP\Services\IOService;
use Symfony\Component\Console\Command\Command;
Expand Down Expand Up @@ -56,6 +57,7 @@ protected function selectServer(string $optionName = 'server', string $promptLab
// Get all servers

$allServers = $this->ensureServersAvailable();

if (is_int($allServers)) {
return $allServers;
}
Expand Down Expand Up @@ -94,21 +96,21 @@ protected function selectServer(string $optionName = 'server', string $promptLab
*/
protected function displayServerDeets(ServerDTO $server, array $sites = []): void
{
$this->io->writeln([
" Name: <fg=gray>{$server->name}</>",
" Host: <fg=gray>{$server->host}</>",
" Port: <fg=gray>{$server->port}</>",
" User: <fg=gray>{$server->username}</>",
' Key: <fg=gray>'.($server->privateKeyPath ?? 'default (~/.ssh/id_ed25519 or ~/.ssh/id_rsa)').'</>',
]);

if (count($sites) > 0) {
$this->io->writeln([' Sites:']);
foreach ($sites as $site) {
$this->io->writeln([" • <fg=gray>{$site->domain}</>"]);
}
$deets = [
'Name' => $server->name,
'Host' => $server->host,
'Port' => $server->port,
'User' => $server->username,
'Key' => $server->privateKeyPath ?? 'default (~/.ssh/id_ed25519 or ~/.ssh/id_rsa)',
];

if (count($sites) > 1) {
$deets['Sites'] = array_map(fn (SiteDTO $site) => $site->domain, $sites);
} elseif (count($sites) === 1) {
$deets['Site'] = $sites[0]->domain;
}

$this->io->displayDeets($deets);
$this->io->writeln('');
}

Expand Down
Loading