refactor: site rename add to create by loadinglucian · Pull Request #113 · loadinglucian/deployer-php

loadinglucian · 2025-12-04T11:16:28Z

Summary by CodeRabbit

New Features
- Optional repo/branch inputs for deploy/site commands; inventory can be updated when provided.
- Post-operation “next steps” guidance shown after server and site actions.
Changes
- Site workflow terminology changed from "Add" → "Create" (site:create); prompts and help updated accordingly.
- Messaging standardized (e.g., "remove" → "delete"); playbook/help texts revised.
- Removed scaffold hello command from CLI.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-04T11:16:38Z

Walkthrough

Renames site "add" to "create" across commands and playbooks, removes HelloCommand, makes SiteDTO repo/branch nullable, adds SiteRepository::update(), enhances SiteDeployCommand with repo/branch resolution and inventory updates, and standardizes messaging and CLI descriptions.

Changes

Cohort / File(s)	Summary
Development Rules Update `\.cursor/rules/00-main.mdc`	Added File Operations subsection, expanded Execution Protocol with STEP BY STEP and ACT steps, and introduced "Test later" guidance.
Console command removed `app/Console/HelloCommand.php`	Deleted HelloCommand class and its registration/imports.
Server commands (metadata & messaging) `app/Console/Server/ServerAddCommand.php`, `app/Console/Server/ServerListCommand.php`, `app/Console/Server/ServerDeleteCommand.php`, `app/Console/Server/ServerInstallCommand.php`, `app/Console/Server/ServerProvisionDigitalOceanCommand.php`	Normalized command descriptions, adjusted deletion/install wording, and added post-add usage hints in ServerAdd flows.
Site create/refactor & registration `app/Console/Site/SiteCreateCommand.php`, `app/SymfonyApp.php`, `app/Console/Site/SiteListCommand.php`, `app/Console/Site/SiteHttpsCommand.php`, `app/Console/Site/SiteDeleteCommand.php`	Renamed SiteAdd → SiteCreate and AsCommand names; removed git options from create; updated SymfonyApp registration; changed messages to use "create"/"delete" and site:create references.
Site deploy enhancements `app/Console/Site/SiteDeployCommand.php`	Added `--repo` and `--branch` options and new `resolveRepoAndBranch()` helper; resolve repo/branch before deploy, update inventory when changed, and add stricter null/empty checks.
DTO nullability `app/DTOs/SiteDTO.php`	Made `repo` and `branch` constructor parameters and properties nullable (`?string`).
Repository updates `app/Repositories/SiteRepository.php`	Added `public function update(SiteDTO $site): void`; refactored hydrate/dehydrate to omit repo/branch when null and to persist nullable values.
Traits & helpers `app/Traits/SitesTrait.php`	Terminology updates (site:add → site:create), stricter comparisons (e.g., `0 === count(...)`, `null ===`), conditional Repo/Branch display, and minor spacing/doc fixes.
Playbooks (rename & messaging) `playbooks/site-create.sh`, `playbooks/site-delete.sh`	Updated playbook descriptions to "create", renamed remove_* functions to delete_*, adjusted call sites and user-facing messages.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Review hotspots:
- app/Console/Site/SiteDeployCommand.php — new CLI options, resolveRepoAndBranch(), inventory update and replay payload changes.
- app/Repositories/SiteRepository.php — new update() and hydrate/dehydrate changes for nullable repo/branch.
- app/DTOs/SiteDTO.php and all call sites — ensure nullable repo/branch handling is safe.
- app/Console/Site/SiteCreateCommand.php & app/Traits/SitesTrait.php — removal of git options and message/validation consistency.
- playbooks/site-delete.sh — renamed functions and their call sites.

Possibly related PRs

refactor: rename namespace to deployer #110 — Modifies the same command and site-related files (HelloCommand removal, SiteAdd→SiteCreate, DTO/repository changes).
feat: site atomic deployments #98 — Overlaps on site deployment flow, SiteDeployCommand and playbook edits.
feat: add site commands #46 — Related migrations of site command names, DTO/repository serialization, and command registration.

Poem

🐰 I hopped from add to create with glee,

Goodbye hello, inventory set free.
Repos nullable, branches take flight,
Playbooks renamed, messages bright,
A rabbit's cheer for tidy code tonight 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: site rename add to create' accurately describes the primary change in the changeset—renaming the site:add command to site:create throughout the codebase and related refactoring.
Docstring Coverage	✅ Passed	Docstring coverage is 97.44% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch refactor/site-rename-add-to-create

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

.cursor/rules/00-main.mdc (1)
60-62: Tighten conversational prose in "Test later" section to imperative mood.

Line 62 ("Tests are something we will focus on...") shifts to conversational explanation rather than maintaining the imperative directive tone required in rule files. Based on learnings, rule files should use imperative mood throughout.

Apply this diff to align with imperative mood:
 ### Test later
 
-Don't run or create or update tests UNLESS explicitly instructed to do so.
-
-Tests are something we will focus on separately from building features.
+Do not run, create, or update tests unless explicitly instructed.
app/Console/Site/SiteHttpsCommand.php (1)
105-105: Inconsistent terminology: "re-add" should be "re-create".

Line 105 still references "re-add the site" which is inconsistent with the PR's terminology refactoring from "add" to "create". For consistency, this should be updated to "re-create the site".

Apply this diff to maintain consistent terminology:
-            $this->nay("Could not detect PHP version for '{$site->domain}' from server config; re-add the site or run server:info to debug.");
+            $this->nay("Could not detect PHP version for '{$site->domain}' from server config; re-create the site or run server:info to debug.");

🧹 Nitpick comments (3)

app/Console/Server/ServerDeleteCommand.php (1)
183-183: Terminology normalized for consistency.

The change aligns the success message with the informational messages on lines 85 and 97, creating consistency within the command flow. While the command name and description still use "delete", the distinction between "delete" (the overall operation) and "remove from inventory" (the specific inventory action) appears intentional.

If full consistency is desired, consider updating line 18 as well:
-    description: 'Delete a server from inventory'
+    description: 'Remove a server from inventory'
app/Console/Site/SiteDeployCommand.php (1)
260-266: Consider conditional inclusion of repo/branch in replay payload.

Per the learning about commandReplay(): "only include conditional options if they were actually used." Since repo and branch are now always included regardless of whether they came from stored values or CLI input, the replay command will always contain these options.

If the intent is for users to replay with minimal options (relying on stored values), consider including repo/branch only when they were explicitly provided via CLI or prompted.
 $this->commandReplay('site:deploy', [
     'domain' => $site->domain,
-    'repo' => $repo,
-    'branch' => $branch,
+    ...($needsUpdate ? ['repo' => $repo, 'branch' => $branch] : []),
     'keep-releases' => $keepReleases,
     'yes' => true,
 ]);
app/Console/Site/SiteCreateCommand.php (1)
180-191: Align validateServerReady docblock wording with “create” terminology

The tightened Caddy availability check (true === ($info['caddy']['available'] ?? false)) is a nice improvement, and the overall validation logic is straightforward.

The docblock summary still says “ready to add site”, which is now out of sync with the command’s terminology. Consider updating for clarity:
-     * Validate that server is ready to add site.
+     * Validate that server is ready to create a site.
Also applies to: 193-206

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b66ea8 and 095ad6f.

📒 Files selected for processing (18)

.cursor/rules/00-main.mdc (1 hunks)
app/Console/HelloCommand.php (0 hunks)
app/Console/Server/ServerAddCommand.php (2 hunks)
app/Console/Server/ServerDeleteCommand.php (1 hunks)
app/Console/Server/ServerInstallCommand.php (1 hunks)
app/Console/Server/ServerListCommand.php (1 hunks)
app/Console/Server/ServerProvisionDigitalOceanCommand.php (1 hunks)
app/Console/Site/SiteCreateCommand.php (13 hunks)
app/Console/Site/SiteDeleteCommand.php (5 hunks)
app/Console/Site/SiteDeployCommand.php (15 hunks)
app/Console/Site/SiteHttpsCommand.php (1 hunks)
app/Console/Site/SiteListCommand.php (1 hunks)
app/DTOs/SiteDTO.php (1 hunks)
app/Repositories/SiteRepository.php (3 hunks)
app/SymfonyApp.php (2 hunks)
app/Traits/SitesTrait.php (10 hunks)
playbooks/site-create.sh (2 hunks)
playbooks/site-delete.sh (4 hunks)

💤 Files with no reviewable changes (1)

app/Console/HelloCommand.php

🧰 Additional context used

📓 Path-based instructions (5)

**/*.php

📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)

**/*.php: Eliminate single-use methods: inline if called once
Cache computed values: initialize expensive calculations in constructor
Avoid method call overhead: direct property access when appropriate
Group related functions into comment-separated sections
Order alphabetically after grouping logically

**/*.php: Follow PSR-12 standard with strict types enabled and use PHP 8.x features (unions, match, attributes, readonly)
Use explicit return types with generics (e.g., Collection<int, User>) for all functions and methods
Use dependency injection via Symfony patterns for all object dependencies
Use Symfony classes over native PHP functions (Filesystem, Process) for testability
Always use Yoda conditions: place constants/literals on the LEFT side of comparisons (e.g., null === $value) to prevent accidental assignment
Always use braces for ALL control structures (if, else, elseif, for, foreach, while, do-while), even for single-line bodies
Use @var annotations to help PHPStan understand types it cannot infer, not assert() in production code
Always add use statements for vendor packages and project classes; root namespace FQDNs acceptable for built-in exceptions (e.g., \InvalidArgumentException)
Use $container->build(ClassName::class) for all object creation; use this except for DTOs, value objects, and pure data structures
Use constructor injection in production code and direct instantiation in tests for container access
All object dependencies must be declared in constructor signatures; NO circular dependencies allowed
Use minimalist DocBlock descriptions with parameters and return types for classes and functions; separate sections visually with one newline between headers/paragraphs

**/*.php: Services and Repositories must throw \RuntimeException with complete, actionable messages that include relevant context (paths, names, IDs, hosts) and are user-facing, not fragments
When catching and re-throwing exceptions in Services and Repositories, use t...

Files:

app/Console/Server/ServerInstallCommand.php
app/Console/Server/ServerProvisionDigitalOceanCommand.php
app/Console/Server/ServerListCommand.php
app/DTOs/SiteDTO.php
app/Console/Server/ServerDeleteCommand.php
app/Console/Server/ServerAddCommand.php
app/Console/Site/SiteDeleteCommand.php
app/Console/Site/SiteListCommand.php
app/Console/Site/SiteHttpsCommand.php
app/Traits/SitesTrait.php
app/Console/Site/SiteCreateCommand.php
app/Console/Site/SiteDeployCommand.php
app/Repositories/SiteRepository.php
app/SymfonyApp.php

**/*Command.php

📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)

Command Layer: handle user interaction and orchestrate Services; contain NO business logic, NO duplicate orchestration, NO invocation of other commands

**/*Command.php: Command layer (Commands and Orchestration Traits) must display caught \RuntimeException messages directly without adding redundant prefixes like 'Failed to...'
Only add contextual information to exception displays in Command layer when necessary: raw output for debugging, actionable troubleshooting steps, or when the exception message is too technical/generic

Files:

app/Console/Server/ServerInstallCommand.php
app/Console/Server/ServerProvisionDigitalOceanCommand.php
app/Console/Server/ServerListCommand.php
app/Console/Server/ServerDeleteCommand.php
app/Console/Server/ServerAddCommand.php
app/Console/Site/SiteDeleteCommand.php
app/Console/Site/SiteListCommand.php
app/Console/Site/SiteHttpsCommand.php
app/Console/Site/SiteCreateCommand.php
app/Console/Site/SiteDeployCommand.php

**/{*Command,*Service}.php

📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)

Only Commands perform console I/O using SymfonyStyle; Services return exceptions/data for Commands to display

Files:

app/Console/Server/ServerInstallCommand.php
app/Console/Server/ServerProvisionDigitalOceanCommand.php
app/Console/Server/ServerListCommand.php
app/Console/Server/ServerDeleteCommand.php
app/Console/Server/ServerAddCommand.php
app/Console/Site/SiteDeleteCommand.php
app/Console/Site/SiteListCommand.php
app/Console/Site/SiteHttpsCommand.php
app/Console/Site/SiteCreateCommand.php
app/Console/Site/SiteDeployCommand.php

**/*.{sh,bash}

📄 CodeRabbit inference engine (.cursor/rules/05-bash.mdc)

**/*.{sh,bash}: Use [[ ... ]] for conditionals instead of [ ... ] or test
Use $(...) for command substitution instead of backticks
Use ((...)) and $((...)) for math operations, never use let
Define functions without function keyword and always use local for variables within functions
Place then on the same line as if, and do on the same line as while in block statements
Prefer parameter expansion over external commands (e.g., use ${0##*/} instead of $(basename "$0"))
Use double quotes for parameter expansions and single quotes for string literals
Exception: Variables controlled by the script (not user input) may be unquoted in [[ ... ]] contexts
Use bash arrays instead of space-separated strings, and use quoted array expansion "${array[@]}"
Always check commands that can fail with || exit or similar error handling
Use set -o pipefail to catch errors in pipelines
Do not use set -e (errexit); prefer explicit error checking instead
Never use eval due to security risks and inability to perform static analysis
Use redirection or built-in read for reading files, stream with while IFS= read loops instead of loading entire files
Never parse ls output for listing files; use shell globs instead
Use tabs for indentation in Bash scripts
Maintain maximum line length of 80 columns in Bash scripts
Use semicolons only in control statements like if true; then
Maintain maximum 1 blank line between sections in Bash scripts
Use shebang #!/usr/bin/env bash at the start of Bash scripts

Files:

playbooks/site-delete.sh
playbooks/site-create.sh

**/*Trait.php

📄 CodeRabbit inference engine (.cursor/rules/04-exceptions.mdc)

**/*Trait.php: Input Validation Traits must provide a validateNameInput() method that returns ?string (error message or null) for prompts/CLI options validation
Heavy I/O Validation Traits must provide a validate*() method that throws \RuntimeException with complete error messages instead of returning ?string

Files:

app/Traits/SitesTrait.php

🧠 Learnings (29)