diff --git a/api/managers.py b/api/managers.py index b9f6335..e5b792f 100644 --- a/api/managers.py +++ b/api/managers.py @@ -95,8 +95,7 @@ service_store_manager = ServiceStoreManager( service_composer=service_composer, ) -setup_manager = SetupManager(config_manager=config_manager, auth_manager=auth_manager, - service_store_manager=service_store_manager) +setup_manager = SetupManager(config_manager=config_manager, auth_manager=auth_manager) # Service logger configuration _service_log_configs = { diff --git a/api/setup_manager.py b/api/setup_manager.py index 482e3a4..a9a01c0 100644 --- a/api/setup_manager.py +++ b/api/setup_manager.py @@ -10,7 +10,6 @@ import fcntl import logging import os import re -import threading from typing import Any, Dict, List logger = logging.getLogger(__name__) @@ -95,10 +94,9 @@ def _build_ddns_config(domain_mode: str, cloudflare_api_token: str = '', class SetupManager: """Manages the first-run setup wizard state and completion.""" - def __init__(self, config_manager, auth_manager, service_store_manager=None): + def __init__(self, config_manager, auth_manager): self.config_manager = config_manager self.auth_manager = auth_manager - self.service_store_manager = service_store_manager # ── state helpers ───────────────────────────────────────────────────── @@ -174,7 +172,6 @@ class SetupManager: domain_mode = payload.get('domain_mode', '') domain_name = payload.get('domain_name', '') timezone = payload.get('timezone', '') - services_enabled = payload.get('services_enabled', []) ddns_provider = payload.get('ddns_provider', 'none') cloudflare_api_token = payload.get('cloudflare_api_token', '') duckdns_token = payload.get('duckdns_token', '') @@ -188,8 +185,6 @@ class SetupManager: ) if not timezone or not isinstance(timezone, str): errors.append('timezone is required.') - if not isinstance(services_enabled, list): - errors.append('services_enabled must be a list.') if errors: return {'success': False, 'errors': errors} @@ -235,7 +230,6 @@ class SetupManager: if domain_name: self.config_manager.set_identity_field('domain_name', domain_name) self.config_manager.set_identity_field('timezone', timezone) - self.config_manager.set_identity_field('services_enabled', services_enabled) self.config_manager.set_identity_field('ddns_provider', ddns_provider) if cloudflare_api_token: self.config_manager.set_identity_field('cloudflare_api_token', cloudflare_api_token) @@ -265,21 +259,6 @@ class SetupManager: # ── mark setup complete (must be last) ───────────────────────── self.config_manager.set_identity_field('setup_complete', True) - # Trigger service installs in background — non-blocking, failures are logged - if services_enabled and self.service_store_manager is not None: - def _install_services(): - for svc_id in services_enabled: - try: - result = self.service_store_manager.install(svc_id) - if result.get('ok'): - logger.info('Wizard: installed service %r', svc_id) - else: - logger.warning('Wizard: install %r failed: %s', - svc_id, result.get('error') or result.get('errors')) - except Exception as exc: - logger.warning('Wizard: install %r raised: %s', svc_id, exc) - threading.Thread(target=_install_services, daemon=True).start() - logger.info(f"Setup completed. cell_name={cell_name!r}, domain_mode={domain_mode!r}") return {'success': True, 'redirect': '/login'} diff --git a/tests/test_setup_manager.py b/tests/test_setup_manager.py index f3e9ff0..e7548c6 100644 --- a/tests/test_setup_manager.py +++ b/tests/test_setup_manager.py @@ -50,7 +50,6 @@ def _valid_payload(**overrides): 'password': 'SecurePass1!', 'domain_mode': 'lan', 'timezone': 'UTC', - 'services_enabled': ['wireguard'], 'ddns_provider': 'none', } base.update(overrides) @@ -203,7 +202,7 @@ def test_complete_setup_calls_set_identity_field_for_each_field( setup_manager.complete_setup(_valid_payload()) calls = mock_config_manager.set_identity_field.call_args_list field_names = [c[0][0] for c in calls] - for expected in ('cell_name', 'domain_mode', 'timezone', 'services_enabled', 'ddns_provider'): + for expected in ('cell_name', 'domain_mode', 'timezone', 'ddns_provider'): assert expected in field_names, f"set_identity_field not called for '{expected}'" diff --git a/tests/test_setup_manager_install_wiring.py b/tests/test_setup_manager_install_wiring.py deleted file mode 100644 index 85d0b3e..0000000 --- a/tests/test_setup_manager_install_wiring.py +++ /dev/null @@ -1,155 +0,0 @@ -"""Tests for SetupManager service-install wiring added in Phase 6. - -Verifies that complete_setup() triggers background service installs when -service_store_manager is provided and services_enabled is non-empty. -""" -import os -import sys -import time -import unittest -from pathlib import Path -from unittest.mock import MagicMock, call, patch - -sys.path.insert(0, str(Path(__file__).parent.parent / 'api')) -from setup_manager import SetupManager - - -# ── helpers ─────────────────────────────────────────────────────────────────── - -def _valid_payload(**overrides): - base = { - 'cell_name': 'mycel', - 'password': 'SecurePass1!', - 'domain_mode': 'lan', - 'timezone': 'UTC', - 'services_enabled': ['email'], - 'ddns_provider': 'none', - } - base.update(overrides) - return base - - -def _make_setup_manager(services_enabled=None, service_store_manager=None): - config_mgr = MagicMock() - config_mgr.get_identity.return_value = {} - auth_mgr = MagicMock() - auth_mgr.create_user.return_value = True - return SetupManager( - config_manager=config_mgr, - auth_manager=auth_mgr, - service_store_manager=service_store_manager, - ), config_mgr, auth_mgr - - -# ── constructor ─────────────────────────────────────────────────────────────── - -class TestSetupManagerConstructor(unittest.TestCase): - - def test_accepts_service_store_manager_kwarg(self): - ssm = MagicMock() - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - self.assertIs(sm.service_store_manager, ssm) - - def test_defaults_service_store_manager_to_none(self): - config_mgr = MagicMock() - config_mgr.get_identity.return_value = {} - auth_mgr = MagicMock() - sm = SetupManager(config_manager=config_mgr, auth_manager=auth_mgr) - self.assertIsNone(sm.service_store_manager) - - -# ── complete_setup + service install wiring ─────────────────────────────────── - -class TestCompleteSetupInstallWiring(unittest.TestCase): - - def test_no_service_store_manager_does_not_crash(self, tmp_path=None): - """service_store_manager=None with services_enabled set must not raise.""" - sm, _, _ = _make_setup_manager(service_store_manager=None) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=['email'])) - self.assertTrue(result['success']) - - def test_empty_services_enabled_no_install_called(self): - """install() must not be called when services_enabled is empty.""" - ssm = MagicMock() - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=[])) - time.sleep(0.05) - ssm.install.assert_not_called() - self.assertTrue(result['success']) - - def test_services_enabled_triggers_install_in_background(self): - """install() must be called with each service id after complete_setup.""" - ssm = MagicMock() - ssm.install.return_value = {'ok': True} - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=['email'])) - self.assertTrue(result['success']) - # Give the daemon thread time to finish - time.sleep(0.1) - ssm.install.assert_called_with('email') - - def test_multiple_services_all_installed(self): - ssm = MagicMock() - ssm.install.return_value = {'ok': True} - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup( - _valid_payload(services_enabled=['email', 'calendar', 'files']) - ) - self.assertTrue(result['success']) - time.sleep(0.1) - installed = [c[0][0] for c in ssm.install.call_args_list] - self.assertIn('email', installed) - self.assertIn('calendar', installed) - self.assertIn('files', installed) - - def test_install_failure_does_not_fail_wizard(self): - """An exception from install() must not propagate to complete_setup.""" - ssm = MagicMock() - ssm.install.side_effect = RuntimeError('Docker daemon unreachable') - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=['email'])) - self.assertTrue(result['success']) - self.assertEqual(result.get('redirect'), '/login') - - def test_install_returning_error_dict_does_not_fail_wizard(self): - """install() returning {'ok': False} must not affect setup result.""" - ssm = MagicMock() - ssm.install.return_value = {'ok': False, 'error': 'image pull failed'} - sm, _, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=['email'])) - self.assertTrue(result['success']) - - def test_complete_setup_still_marks_setup_complete_before_install(self): - """setup_complete must be persisted even if install thread races.""" - ssm = MagicMock() - ssm.install.return_value = {'ok': True} - sm, config_mgr, _ = _make_setup_manager(service_store_manager=ssm) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - sm.complete_setup(_valid_payload(services_enabled=['email'])) - # setup_complete must be the last set_identity_field call - calls = config_mgr.set_identity_field.call_args_list - last = calls[-1] - self.assertEqual(last, call('setup_complete', True)) - - def test_service_store_manager_none_with_empty_services_succeeds(self): - sm, _, _ = _make_setup_manager(service_store_manager=None) - with patch.dict(os.environ, {'DATA_DIR': self._tmp()}): - result = sm.complete_setup(_valid_payload(services_enabled=[])) - self.assertTrue(result['success']) - - # ── helpers ─────────────────────────────────────────────────────────────── - - def _tmp(self): - """Return a writable temp dir path string for DATA_DIR.""" - import tempfile - return tempfile.mkdtemp() - - -if __name__ == '__main__': - unittest.main() diff --git a/webui/src/pages/Setup.jsx b/webui/src/pages/Setup.jsx index 1da4d9f..2968b2e 100644 --- a/webui/src/pages/Setup.jsx +++ b/webui/src/pages/Setup.jsx @@ -5,7 +5,7 @@ import { setupAPI } from '../services/api'; // ── constants ───────────────────────────────────────────────────────────────── -const TOTAL_STEPS = 5; +const TOTAL_STEPS = 4; const CELL_NAME_RE = /^[a-z][a-z0-9-]{1,30}$/; const DOMAIN_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$/i; @@ -38,19 +38,6 @@ const DOMAIN_OPTIONS = [ }, ]; -const OPTIONAL_SERVICES = [ - { key: 'email', label: 'Email', description: 'Postfix + Dovecot IMAP/SMTP server.' }, - { key: 'calendar', label: 'Calendar & Contacts', description: 'CalDAV/CardDAV via Radicale.' }, - { key: 'files', label: 'Files (WebDAV)', description: 'WebDAV file storage accessible from any device.' }, - { key: 'webmail', label: 'Webmail UI', description: 'Browser-based email client (Roundcube).' }, -]; - -const ALWAYS_ON_SERVICES = [ - { key: 'vpn', label: 'VPN (WireGuard)' }, - { key: 'dns', label: 'DNS (CoreDNS)' }, - { key: 'api', label: 'API (cell-api)' }, -]; - // ── helpers ─────────────────────────────────────────────────────────────────── function getAllTimezones() { @@ -513,48 +500,7 @@ function Step3Timezone({ value, onChange, onNext, onBack }) { ); } -// ── step 4: services ────────────────────────────────────────────────────────── - -function Step4Services({ selected, onChange, onNext, onBack }) { - const toggle = key => onChange(selected.includes(key) ? selected.filter(k => k !== key) : [...selected, key]); - - return ( -
- -
- {OPTIONAL_SERVICES.map(svc => { - const checked = selected.includes(svc.key); - return ( - - ); - })} -
-
-

Always enabled

-
- {ALWAYS_ON_SERVICES.map(svc => ( -
- - {svc.label} - always enabled -
- ))} -
-
- -
- ); -} - -// ── step 5: review ──────────────────────────────────────────────────────────── +// ── step 4: review ──────────────────────────────────────────────────────────── function ReviewRow({ label, value }) { return ( @@ -565,23 +511,19 @@ function ReviewRow({ label, value }) { ); } -function Step5Review({ domainType, domainName, services, timezone, onBack, onSubmit, submitting, submitError }) { +function Step4Review({ domainType, domainName, timezone, onBack, onSubmit, submitting, submitError }) { const domainDisplay = domainName || 'LAN only (no public domain)'; const providerLabel = DOMAIN_OPTIONS.find(o => o.value === domainType)?.label || domainType; - const serviceLabels = services.length - ? services.map(k => OPTIONAL_SERVICES.find(s => s.key === k)?.label || k).join(', ') - : 'None selected'; return (
-
-
{submitError && (
@@ -614,7 +556,6 @@ export default function Setup() { const [timezone, setTimezone] = useState( (() => { try { return Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; } catch { return 'UTC'; } })() ); - const [services, setServices] = useState([]); const [submitting, setSubmitting] = useState(false); const [submitError, setSubmitError] = useState(''); @@ -648,13 +589,12 @@ export default function Setup() { const ddnsProvider = domainType === 'lan' ? 'none' : domainType === 'http01' ? 'none' : domainType; const payload = { - cell_name: cellName, + cell_name: cellName, password, - domain_mode: domainType, - domain_name: domainName, + domain_mode: domainType, + domain_name: domainName, timezone, - services_enabled: services, - ddns_provider: ddnsProvider, + ddns_provider: ddnsProvider, ...(domainType === 'cloudflare' && { cloudflare_api_token: cloudflareToken }), ...(domainType === 'duckdns' && { duckdns_token: duckdnsToken }), }; @@ -720,12 +660,9 @@ export default function Setup() { )} {step === 4 && ( - - )} - {step === 5 && ( -