Commit 49614f5
committed
Merge #83: Refactor handle_environment_creation() using Steps Pattern
c8931b0 refactor: extract helper functions from handle_environment_creation (copilot-swe-agent[bot])
3e742ed Initial plan (copilot-swe-agent[bot])
Pull request description:
The `handle_environment_creation()` function mixed configuration loading, command execution, and result display in a single 58-line function, violating SRP and making individual steps untestable.
## Changes
**Main function refactored to orchestration-only (21 lines):**
```rust
pub fn handle_environment_creation(
env_file: &Path,
working_dir: &Path,
) -> Result<(), CreateSubcommandError> {
let mut ctx = CommandContext::new(working_dir.to_path_buf());
// Step 1: Load configuration
let config = load_configuration(ctx.output(), env_file)?;
// Step 2: Execute command
let environment = {
let repository = ctx.repository().clone();
let clock = ctx.clock().clone();
execute_create_command(ctx.output(), config, repository, clock)?
};
// Step 3: Display results
display_creation_results(ctx.output(), &environment);
Ok(())
}
```
**Extracted three focused helper functions:**
- `load_configuration()` - Config loading and validation
- `execute_create_command()` - Command handler creation and execution
- `display_creation_results()` - Success message formatting
**Added unit tests for each helper function:**
- 3 tests for configuration loading (valid config, missing file, invalid JSON)
- 2 tests for command execution (success, duplicate environment)
- 1 test for result display
- All 5 existing integration tests remain unchanged and passing
No behavioral changes. Each step is now independently testable while maintaining the same user-facing behavior.
<!-- START COPILOT CODING AGENT SUFFIX -->
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>Refactor handle_environment_creation() Using Steps Pattern</issue_title>
> <issue_description>**Parent Issue**: #63
> **Type**: 🔨 Structural Improvement
> **Impact**: 🟢🟢🟢 High
> **Effort**: 🔵🔵🔵 High
> **Priority**: P1
> **Depends On**: #67 (Proposal 4)
>
> ## Problem
>
> The `handle_environment_creation()` function is 110+ lines and does too much:
>
> ```rust
> fn handle_environment_creation(env_file: &Path, working_dir: &Path)
> -> Result<(), CreateSubcommandError>
> {
> // 1. Create user output
> // 2. Load configuration
> // 3. Validate configuration
> // 4. Create repository
> // 5. Create clock
> // 6. Create command handler
> // 7. Execute command
> // 8. Display results
> // All in one function! 110+ lines
> }
> ```
>
> This violates SRP and makes the function hard to test and understand.
>
> ## Proposed Solution
>
> Break down into focused step functions:
>
> ```rust
> pub fn handle_environment_creation(
> env_file: &Path,
> working_dir: &Path,
> ) -> Result<(), CreateSubcommandError> {
> let mut output = output::create_default();
>
> // Step 1: Load configuration
> let config = load_configuration(&mut output, env_file)?;
>
> // Step 2: Create dependencies
> let context = CommandContext::new(working_dir.to_path_buf());
>
> // Step 3: Execute command
> let environment = execute_create_command(&mut output, config, &context)?;
>
> // Step 4: Display results
> display_creation_results(&mut output, &environment);
>
> Ok(())
> }
>
> fn load_configuration(output: &mut UserOutput, env_file: &Path)
> -> Result<EnvironmentCreationConfig, CreateSubcommandError>
> {
> // Focused on configuration loading only
> }
>
> fn execute_create_command(
> output: &mut UserOutput,
> config: EnvironmentCreationConfig,
> context: &CommandContext,
> ) -> Result<Environment, CreateSubcommandError>
> {
> // Focused on command execution only
> }
>
> fn display_creation_results(output: &mut UserOutput, environment: &Environment) {
> // Focused on result display only
> }
> ```
>
> ## Benefits
>
> - ✅ Main function reduced from 110+ to ~25 lines
> - ✅ Each step is self-contained and testable
> - ✅ Clear orchestration flow
> - ✅ Easier to modify individual steps
> - ✅ Better error handling with clear error context
>
> ## Implementation Checklist
>
> - [ ] Create helper function `load_configuration()`
> - [ ] Create helper function `execute_create_command()`
> - [ ] Create helper function `display_creation_results()`
> - [ ] Refactor main `handle_environment_creation()` to orchestrate steps
> - [ ] Add comprehensive documentation to each function
> - [ ] Write unit tests for each helper function
> - [ ] Update integration tests to verify end-to-end behavior
> - [ ] Verify all existing tests still pass
> - [ ] Run linter and fix issues
>
> ## Acceptance Criteria
>
> - [ ] `handle_environment_creation()` is orchestration-only (~25 lines)
> - [ ] Each step has its own focused function
> - [ ] All steps have unit tests
> - [ ] Integration tests verify end-to-end flow
> - [ ] No behavioral changes from user perspective
> - [ ] All tests pass: `cargo test create::subcommands::environment`
> - [ ] Pre-commit checks pass: `./scripts/pre-commit.sh`
> - [ ] Code follows project conventions
>
> ## Related Documentation
>
> - [Module Organization Guide](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/contributing/module-organization.md)
> - [Refactor Plan](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/refactors/plans/presentation-commands-cleanup.md)</issue_description>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> </comments>
>
</details>
- Fixes #69
<!-- START COPILOT CODING AGENT TIPS -->
---
💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
ACKs for top commit:
josecelano:
ACK c8931b0
Tree-SHA512: 7f46bfb7ee88e958ebd59a107ce0dd7abf12f337f4c16f0dfc929a463eabaa08c9c9e986deda35cdecc449feb25117759b0182a891fa38ac8aa5587b97a965f51 file changed
Lines changed: 334 additions & 38 deletions
0 commit comments