feat(create): add interactive prompting and connection checking for system management commands

.changeset/interactive-system-prompts.md

@@ -0,0 +1,11 @@
+---
+"@sap-ux/create": minor
+---
+
+FEAT: Add interactive prompting and connection checking for system management commands
+
+- Added interactive prompts for `add system`, `update system`, and `remove system` commands when flags are not provided
+- Added connection verification for `add system` and `update system` before saving (with `--skip-check` flag to bypass)
+- Added confirmation prompt for `remove system` (with `--force` flag to bypass)
+- All commands now support both flag-based and fully interactive modes
+- No breaking changes - all existing flag-based usage continues to work

CODE_REVIEW_CHANGES_SUMMARY.md

@@ -0,0 +1,164 @@
+# Code Review Changes Summary
+
+## Overview
+Successfully addressed all code review comments from Iain O'Farrell on PR #4949 for the open-ux-tools repository.
+
+## Changes Made
+
+### 1. ✅ Reverted odata-service-inquirer Changes
+**File:** `packages/odata-service-inquirer/src/prompts/datasources/sap-system/validators.ts`
+
+- **Issue:** Unnecessarily extracted a one-liner function
+- **Fix:** Reverted to use the shared function from `@sap-ux/inquirer-common`
+- **Impact:** Maintains code reusability and avoids duplication
+
+### 2. ✅ Removed Connection Type Filtering
+**File:** `packages/inquirer-common/src/validators/system-name-validator.ts`
+
+- **Issue:** System names were filtered by connection type, allowing duplicates across different connection types
+- **Fix:** Removed filtering to enforce global uniqueness
+- **Changes:**
+  - Removed `connectionTypes` parameter from `SystemNameValidationOptions`
+  - Changed from `service.getAll({ backendSystemFilter: { connectionType: [...] }})` to `service.getAll()`
+  - Updated documentation to clarify global uniqueness check
+- **Impact:** Prevents confusing duplicate system names for users
+
+### 3. ✅ Renamed Function
+**File:** Multiple files across packages
+
+- **Issue:** `isSystemNameTaken` was an odd name
+- **Fix:** Renamed to `systemNameExists` for better clarity
+- **Files Updated:**
+  - `packages/inquirer-common/src/index.ts`
+  - `packages/inquirer-common/src/validators/system-name-validator.ts`
+  - `packages/odata-service-inquirer/src/prompts/datasources/sap-system/validators.ts`
+  - `packages/create/src/cli/add/system.ts`
+  - `packages/create/src/cli/update/system.ts`
+  - `packages/create/src/cli/utils/system-prompts.ts`
+  - `packages/create/test/unit/cli/utils/system-prompts.test.ts`
+
+### 4. ✅ Fixed Lint Issues
+**File:** `packages/inquirer-common/src/validators/system-name-validator.ts`
+
+- Fixed `@throws` JSDoc to include type: `@throws {Error}`
+- Changed `||` to `??` for nullish coalescing operator
+
+## Validation Results
+
+### ✅ Linting
+- **inquirer-common:** 2 warnings fixed in system-name-validator.ts, remaining warnings are pre-existing
+- **odata-service-inquirer:** Clean (no errors)
+- **create:** Clean (no errors)
+
+### ✅ SonarQube / SonarJS
+- Configuration exists in `sonar-project.properties` for CI/SonarCloud
+- `eslint-plugin-sonarjs` is configured and active in `eslint.config.mjs`
+- Active rules: `sonarjs/cognitive-complexity`, `sonarjs/no-nested-template-literals`, `sonarjs/no-implicit-dependencies`
+- **Result:** All 58 warnings in modified files are pre-existing issues, **no new SonarJS issues introduced by our changes**
+- Our changes:
+  - ✅ No cognitive complexity issues
+  - ✅ No duplicate strings
+  - ✅ No identical functions
+  - ✅ No implicit dependencies
+
+### ✅ Test Coverage
+- **create package:** 94.47% statements, 88.17% branch, 93.68% functions
+- **system-prompts.ts:** 93.5% coverage (our modified file)
+- All 249 tests passing across 33 test suites
+
+### ✅ Build
+- All affected packages built successfully:
+  - `@sap-ux/inquirer-common`
+  - `@sap-ux/odata-service-inquirer`
+  - `@sap-ux/create`
+
+### ✅ Manual Testing (CLI Commands)
+Tested against built CLI (`node dist/index.js`):
+
+1. **Add system with unique name** ✅
+   ```bash
+   node dist/index.js add system --name "Test System 1" --url https://test1.example.com --skip-check
+   ```
+   Result: System added successfully
+
+2. **Duplicate name validation** ✅
+   ```bash
+   node dist/index.js add system --name "Test System 1" --url https://test2.example.com --skip-check
+   ```
+   Result: Error - "A system with the name 'Test System 1' already exists. Please choose a different name."
+
+3. **Case-insensitive validation** ✅
+   ```bash
+   node dist/index.js add system --name "test system 1" --url https://test2.example.com --skip-check
+   ```
+   Result: Error - "A system with the name 'test system 1' already exists. Please choose a different name."
+
+4. **Add another system** ✅
+   ```bash
+   node dist/index.js add system --name "Test System 2" --url https://test2.example.com --skip-check --client "100"
+   ```
+   Result: System added successfully
+
+5. **List systems** ✅
+   ```bash
+   node dist/index.js list system
+   ```
+   Result: Both test systems shown with correct details
+
+6. **Update system name** ✅
+   ```bash
+   node dist/index.js update system --url https://test1.example.com --name "Test System Updated" --skip-check
+   ```
+   Result: System updated successfully
+
+7. **Prevent update to duplicate name** ✅
+   ```bash
+   node dist/index.js update system --url https://test2.example.com --name "Test System Updated" --skip-check
+   ```
+   Result: Error - "A system with the name 'Test System Updated' already exists. Please choose a different name."
+
+8. **Allow keeping same name on update** ✅
+   ```bash
+   node dist/index.js update system --url https://test1.example.com --name "Test System Updated" --skip-check
+   ```
+   Result: System updated successfully (excludeSystem logic works)
+
+9. **Remove systems** ✅
+   ```bash
+   node dist/index.js remove system --url https://test1.example.com --force
+   node dist/index.js remove system --url https://test2.example.com --client "100" --force
+   ```
+   Result: Both systems removed successfully
+
+## Key Implementation Details
+
+### System Name Validation Logic
+The `systemNameExists` function now:
+- Checks ALL systems globally (no connection type filtering)
+- Performs case-insensitive comparison
+- Trims whitespace before comparison
+- Supports `excludeSystem` option for update operations (allows keeping the same name)
+
+### Global Uniqueness Rationale
+Per Iain's comment: "If you filter you can end up with duplicate system names. If there are multiple systems listed with the same name it would be confusing for users."
+
+Different clients are treated as different connections (different system identifiers), but system names must be globally unique to avoid user confusion.
+
+## Files Modified
+- `packages/inquirer-common/src/index.ts`
+- `packages/inquirer-common/src/validators/system-name-validator.ts`
+- `packages/odata-service-inquirer/src/prompts/datasources/sap-system/validators.ts`
+- `packages/create/src/cli/add/system.ts`
+- `packages/create/src/cli/update/system.ts`
+- `packages/create/src/cli/utils/system-prompts.ts`
+- `packages/create/test/unit/cli/utils/system-prompts.test.ts`
+
+## Summary
+All code review comments have been addressed:
+- ✅ Reverted unnecessary extraction in odata-service-inquirer
+- ✅ Removed connection type filtering for global name uniqueness
+- ✅ Renamed function for better clarity
+- ✅ Fixed lint issues
+- ✅ All tests passing
+- ✅ Manual testing confirms correct behavior
+- ✅ Ready for re-review