|
| 1 | +# Refactor Presentation Commands to Modular Directory Structure |
| 2 | + |
| 3 | +**Issue**: #58 |
| 4 | +**Type**: Refactoring |
| 5 | +**Related**: Follows pattern established in `CreateCommand` (see `src/presentation/commands/create/`) |
| 6 | + |
| 7 | +## Overview |
| 8 | + |
| 9 | +Refactor the existing single-file presentation command (`destroy`) to follow the new modular directory structure pattern established in `CreateCommand`. This improves code organization, maintainability, and consistency by separating concerns into dedicated files. |
| 10 | + |
| 11 | +Currently, the `destroy` command uses a single-file pattern: |
| 12 | + |
| 13 | +- `destroy.rs` (290 lines) - Mixing subcommand logic, error types, and handler orchestration |
| 14 | + |
| 15 | +The `CreateCommand` demonstrates a better structure with separate files for subcommand logic, configuration loading, errors, and tests. |
| 16 | + |
| 17 | +## Goals |
| 18 | + |
| 19 | +- [ ] Migrate `DestroyCommand` to modular structure |
| 20 | +- [ ] Improve code maintainability and readability |
| 21 | +- [ ] Make test organization clearer and more scalable |
| 22 | +- [ ] Establish consistent patterns across all presentation commands |
| 23 | +- [ ] Enable easier addition of configuration loading if needed in the future |
| 24 | + |
| 25 | +## 🏗️ Architecture Requirements |
| 26 | + |
| 27 | +**DDD Layer**: Presentation Layer |
| 28 | +**Module Path**: `src/presentation/commands/destroy/` |
| 29 | +**Pattern**: CLI Subcommand with modular organization |
| 30 | + |
| 31 | +### Module Structure Requirements |
| 32 | + |
| 33 | +- [ ] Follow DDD layer separation (see [docs/codebase-architecture.md](../docs/codebase-architecture.md)) |
| 34 | +- [ ] Maintain existing presentation layer responsibilities |
| 35 | +- [ ] Use appropriate module organization (see [docs/contributing/module-organization.md](../docs/contributing/module-organization.md)) |
| 36 | +- [ ] Preserve existing public API surface (no breaking changes to CLI interface) |
| 37 | + |
| 38 | +### Architectural Constraints |
| 39 | + |
| 40 | +- [ ] No changes to command business logic (refactoring only) |
| 41 | +- [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../docs/contributing/error-handling.md)) |
| 42 | +- [ ] Testing strategy aligns with layer responsibilities (see [docs/contributing/testing.md](../docs/contributing/testing.md)) |
| 43 | +- [ ] All existing tests must continue to pass |
| 44 | +- [ ] Preserve user output and progress reporting features |
| 45 | + |
| 46 | +**Note**: Breaking changes to import paths are acceptable since this library is not yet publicly used. Simplifying the module structure takes priority over backward compatibility. |
| 47 | + |
| 48 | +### Anti-Patterns to Avoid |
| 49 | + |
| 50 | +- ❌ Changing business logic during refactoring |
| 51 | +- ❌ Mixing refactoring with new features |
| 52 | +- ❌ Creating incomplete module structures |
| 53 | +- ❌ Moving presentation logic into application layer |
| 54 | + |
| 55 | +## Specifications |
| 56 | + |
| 57 | +### Current Structure (Single-File Pattern) |
| 58 | + |
| 59 | +```text |
| 60 | +src/presentation/commands/ |
| 61 | +├── create/ # Modular pattern (371 lines subcommand + separate files) |
| 62 | +│ ├── mod.rs |
| 63 | +│ ├── subcommand.rs |
| 64 | +│ ├── config_loader.rs |
| 65 | +│ ├── errors.rs |
| 66 | +│ └── tests/ |
| 67 | +├── destroy.rs # Single file (290 lines) |
| 68 | +└── mod.rs |
| 69 | +``` |
| 70 | + |
| 71 | +**Problems**: |
| 72 | + |
| 73 | +- Mixed concerns: subcommand logic, errors, all in one file |
| 74 | +- Inconsistent with `create/` command structure |
| 75 | +- Harder to add features like configuration file support |
| 76 | +- Test organization unclear |
| 77 | + |
| 78 | +### Target Structure (Modular Pattern) |
| 79 | + |
| 80 | +Based on the `create/` command pattern: |
| 81 | + |
| 82 | +```text |
| 83 | +src/presentation/commands/ |
| 84 | +├── create/ |
| 85 | +│ ├── mod.rs |
| 86 | +│ ├── subcommand.rs |
| 87 | +│ ├── config_loader.rs |
| 88 | +│ ├── errors.rs |
| 89 | +│ └── tests/ |
| 90 | +├── destroy/ |
| 91 | +│ ├── mod.rs # Module documentation & public API |
| 92 | +│ ├── subcommand.rs # Main command handler |
| 93 | +│ ├── errors.rs # Error types with .help() |
| 94 | +│ └── tests/ # Test organization |
| 95 | +│ ├── mod.rs |
| 96 | +│ ├── integration.rs |
| 97 | +│ └── fixtures.rs (if needed) |
| 98 | +└── mod.rs |
| 99 | +``` |
| 100 | + |
| 101 | +### File Responsibilities |
| 102 | + |
| 103 | +#### `mod.rs` (Module Documentation & Public API) |
| 104 | + |
| 105 | +- Module-level documentation |
| 106 | +- Re-exports public types (`pub use`) |
| 107 | +- Declares submodules |
| 108 | +- Serves as clean entry point |
| 109 | + |
| 110 | +**Example** (based on `create/mod.rs`): |
| 111 | + |
| 112 | +```rust |
| 113 | +//! Destroy Command Presentation Module |
| 114 | +//! |
| 115 | +//! This module implements the CLI presentation layer for the destroy command, |
| 116 | +//! handling argument processing and user interaction. |
| 117 | + |
| 118 | +pub mod errors; |
| 119 | +pub mod subcommand; |
| 120 | + |
| 121 | +#[cfg(test)] |
| 122 | +mod tests; |
| 123 | + |
| 124 | +// Re-export commonly used types for convenience |
| 125 | +pub use errors::DestroySubcommandError; |
| 126 | +pub use subcommand::handle_destroy_command; |
| 127 | +``` |
| 128 | + |
| 129 | +#### `subcommand.rs` (Main Command Handler) |
| 130 | + |
| 131 | +- Subcommand orchestration logic |
| 132 | +- Environment validation |
| 133 | +- Repository initialization |
| 134 | +- Command handler invocation |
| 135 | +- User output and progress reporting |
| 136 | + |
| 137 | +**Size**: 150-200 lines typical |
| 138 | + |
| 139 | +#### `errors.rs` (Error Types) |
| 140 | + |
| 141 | +- Error enum definitions using `thiserror` |
| 142 | +- `.help()` methods for actionable error messages |
| 143 | +- Error documentation |
| 144 | +- Presentation-layer specific error handling |
| 145 | + |
| 146 | +**Size**: 80-100 lines typical |
| 147 | + |
| 148 | +#### `tests/` (Test Organization) |
| 149 | + |
| 150 | +- `mod.rs`: Test module entry, re-exports |
| 151 | +- `integration.rs`: Integration test cases |
| 152 | +- `fixtures.rs`: Test fixtures (if needed) |
| 153 | + |
| 154 | +### Migration Strategy |
| 155 | + |
| 156 | +1. **Create Directory Structure** |
| 157 | + |
| 158 | + ```bash |
| 159 | + mkdir -p src/presentation/commands/destroy/tests |
| 160 | + ``` |
| 161 | + |
| 162 | +2. **Split Files** |
| 163 | + |
| 164 | + - Extract errors → `errors.rs` |
| 165 | + - Move subcommand handler → `subcommand.rs` |
| 166 | + - Create `mod.rs` with documentation |
| 167 | + - Organize tests → `tests/` |
| 168 | + |
| 169 | +3. **Update Imports Throughout Codebase** |
| 170 | + |
| 171 | + - Update parent `mod.rs` to use new module structure |
| 172 | + - Fix ALL imports in CLI/main that use the destroy command |
| 173 | + - Update any test imports |
| 174 | + - **Breaking changes acceptable** - simplify paths where possible |
| 175 | + |
| 176 | +4. **Verify** |
| 177 | + |
| 178 | + - Run `cargo test` - all tests must pass |
| 179 | + - Run `cargo clippy` - no new warnings |
| 180 | + - Run `./scripts/pre-commit.sh` - all checks pass |
| 181 | + |
| 182 | +## Implementation Plan |
| 183 | + |
| 184 | +**Important**: This is a single-command refactoring. Run pre-commit checks and commit when complete. |
| 185 | + |
| 186 | +### Subtask 1: Migrate DestroyCommand (2-3 hours) |
| 187 | + |
| 188 | +- [ ] Create `src/presentation/commands/destroy/` directory |
| 189 | +- [ ] Create `destroy/mod.rs` with module documentation and re-exports |
| 190 | +- [ ] Create `destroy/subcommand.rs` - move handler logic from `destroy.rs` |
| 191 | +- [ ] Create `destroy/errors.rs` - extract error types from `destroy.rs` |
| 192 | +- [ ] Create `destroy/tests/mod.rs` - test module entry point |
| 193 | +- [ ] Create `destroy/tests/integration.rs` - move existing tests (if any) |
| 194 | +- [ ] Update `src/presentation/commands/mod.rs` to use new structure |
| 195 | +- [ ] **Update ALL imports across codebase** (CLI, main, other modules) |
| 196 | +- [ ] Delete old `destroy.rs` file |
| 197 | +- [ ] Run `cargo test` to verify all tests pass |
| 198 | +- [ ] Run `cargo clippy` to check for issues |
| 199 | +- [ ] Run `./scripts/pre-commit.sh` to verify quality |
| 200 | +- [ ] **Commit this refactoring** |
| 201 | + |
| 202 | +### Subtask 2: Final Verification and Documentation (30 minutes) |
| 203 | + |
| 204 | +- [ ] Verify destroy command structure matches create/ pattern |
| 205 | +- [ ] Update any relevant documentation mentioning command structure |
| 206 | +- [ ] Ensure all imports across the codebase are correct |
| 207 | +- [ ] Final pre-commit verification: `./scripts/pre-commit.sh` |
| 208 | +- [ ] **Final commit with any documentation updates** |
| 209 | + |
| 210 | +## Acceptance Criteria |
| 211 | + |
| 212 | +**Quality Checks**: |
| 213 | + |
| 214 | +- [ ] Pre-commit checks pass: `./scripts/pre-commit.sh` |
| 215 | + |
| 216 | +**Refactoring-Specific Criteria**: |
| 217 | + |
| 218 | +- [ ] Destroy command uses modular directory structure |
| 219 | +- [ ] Directory structure matches `create/` pattern exactly |
| 220 | +- [ ] All existing tests continue to pass |
| 221 | +- [ ] No changes to business logic (pure refactoring) |
| 222 | +- [ ] File sizes are manageable (< 250 lines per file) |
| 223 | +- [ ] Clear separation of concerns (subcommand, errors, tests) |
| 224 | +- [ ] Module documentation is comprehensive |
| 225 | +- [ ] Error types include `.help()` methods |
| 226 | +- [ ] Test organization is clear and scalable |
| 227 | +- [ ] All imports updated correctly throughout entire codebase |
| 228 | + |
| 229 | +**Import Updates**: |
| 230 | + |
| 231 | +- [ ] All imports in CLI/main updated |
| 232 | +- [ ] All imports in tests updated |
| 233 | +- [ ] Parent `mod.rs` correctly re-exports types for convenience |
| 234 | + |
| 235 | +**Structure Verification**: |
| 236 | + |
| 237 | +- [ ] Destroy command has `mod.rs`, `subcommand.rs`, `errors.rs` |
| 238 | +- [ ] Has `tests/` subdirectory with `mod.rs`, `integration.rs` |
| 239 | +- [ ] Parent `mod.rs` correctly imports and re-exports destroy command |
| 240 | +- [ ] No orphaned files or unused code |
| 241 | + |
| 242 | +**Commit Strategy**: |
| 243 | + |
| 244 | +- [ ] Refactoring committed independently |
| 245 | +- [ ] Pre-commit checks pass before commit |
| 246 | +- [ ] Clear, descriptive commit message following conventional commits format |
| 247 | +- [ ] Documentation commit separate if needed |
| 248 | + |
| 249 | +## Related Documentation |
| 250 | + |
| 251 | +- [Module Organization Guide](../contributing/module-organization.md) - File organization conventions |
| 252 | +- [Error Handling Guide](../contributing/error-handling.md) - Error type patterns and `.help()` methods |
| 253 | +- [Testing Conventions](../contributing/testing.md) - Test organization and naming |
| 254 | +- [Codebase Architecture](../codebase-architecture.md) - DDD layer structure |
| 255 | +- Reference Implementation: `src/presentation/commands/create/` - Example of target structure |
| 256 | + |
| 257 | +## Benefits |
| 258 | + |
| 259 | +### Maintainability ✅ |
| 260 | + |
| 261 | +- **Smaller files**: Easier to navigate (150-200 lines vs 290 lines) |
| 262 | +- **Single responsibility**: Each file has one clear purpose |
| 263 | +- **Easier reviews**: Changes to errors don't require reviewing subcommand logic |
| 264 | + |
| 265 | +### Scalability ✅ |
| 266 | + |
| 267 | +- **Room to grow**: Can add config_loader or validators without bloating main file |
| 268 | +- **Test organization**: Can add more test modules easily |
| 269 | +- **Better IDE support**: Smaller files improve code navigation |
| 270 | + |
| 271 | +### Consistency ✅ |
| 272 | + |
| 273 | +- **Uniform patterns**: All presentation commands follow same structure |
| 274 | +- **Predictable locations**: Developers know where to find subcommand vs errors vs tests |
| 275 | +- **Easier onboarding**: New contributors understand structure quickly |
| 276 | + |
| 277 | +### Testing ✅ |
| 278 | + |
| 279 | +- **Dedicated test organization**: Clear separation of test types |
| 280 | +- **Integration tests isolated**: Easier to maintain and extend |
| 281 | +- **Better test discoverability**: Tests organized by purpose |
| 282 | + |
| 283 | +## Notes |
| 284 | + |
| 285 | +### Why Start with Destroy (Not Other Commands) |
| 286 | + |
| 287 | +We're migrating `destroy` first because: |
| 288 | + |
| 289 | +1. It's the only other presentation command besides `create` |
| 290 | +2. Single command makes this a straightforward refactoring |
| 291 | +3. Once complete, all presentation commands will be consistent |
| 292 | +4. Other commands (provision, configure) don't exist in presentation layer yet |
| 293 | + |
| 294 | +### Future Commands |
| 295 | + |
| 296 | +When adding new presentation commands (provision, configure, etc.): |
| 297 | + |
| 298 | +- Start with modular structure from the beginning |
| 299 | +- Follow the `create/` and `destroy/` patterns |
| 300 | +- Include `subcommand.rs`, `errors.rs`, `tests/` from the start |
| 301 | + |
| 302 | +### Breaking Changes: Acceptable |
| 303 | + |
| 304 | +**Important**: Since this library is not yet publicly used, breaking changes to import paths are acceptable and even encouraged if they simplify the refactoring: |
| 305 | + |
| 306 | +- ✅ Import paths will change (e.g., `use crate::presentation::commands::destroy::handle_destroy_command`) |
| 307 | +- ✅ Can simplify re-exports if it improves clarity |
| 308 | +- ✅ Focus on best final structure, not backward compatibility |
| 309 | +- ⚠️ Business logic must remain unchanged (pure refactoring) |
| 310 | +- ⚠️ All existing tests must still pass (behavior unchanged) |
| 311 | + |
| 312 | +This freedom allows us to: |
| 313 | + |
| 314 | +- Create the cleanest possible module structure |
| 315 | +- Avoid complex re-export chains just for compatibility |
| 316 | +- Optimize for maintainability over backward compatibility |
| 317 | + |
| 318 | +### Preserving Git History |
| 319 | + |
| 320 | +Consider using `git mv` where possible to preserve file history: |
| 321 | + |
| 322 | +```bash |
| 323 | +# When creating new files, use git mv for better history tracking |
| 324 | +git mv src/presentation/commands/destroy.rs \ |
| 325 | + src/presentation/commands/destroy/subcommand.rs |
| 326 | +``` |
| 327 | + |
| 328 | +However, since we're splitting files, manual creation may be clearer. |
| 329 | + |
| 330 | +### Related to Command Handlers Refactoring |
| 331 | + |
| 332 | +This refactoring is complementary to issue #56 (Command Handlers refactoring): |
| 333 | + |
| 334 | +- Issue #56: Application layer command handlers |
| 335 | +- This issue: Presentation layer commands |
| 336 | +- Both establish modular patterns for their respective layers |
| 337 | +- Can be implemented independently or in parallel |
0 commit comments