feat: remove optional services step from setup wizard
Services are now installed post-setup from the Store page, so the wizard step that let users pre-select email/calendar/files is removed. Reduces wizard from 5 steps to 4 (Step4Services deleted, Step5Review renamed to Step4Review). Backend drops services_enabled validation, background install thread, and service_store_manager dependency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-2
@@ -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 = {
|
||||
|
||||
+1
-22
@@ -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'}
|
||||
|
||||
|
||||
@@ -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}'"
|
||||
|
||||
|
||||
|
||||
@@ -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()
|
||||
+10
-73
@@ -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 (
|
||||
<div>
|
||||
<StepHeader step={4} title="Optional services" description="Choose which services to enable. You can change this later in Settings." />
|
||||
<div className="space-y-2 mb-6">
|
||||
{OPTIONAL_SERVICES.map(svc => {
|
||||
const checked = selected.includes(svc.key);
|
||||
return (
|
||||
<label key={svc.key} className={`flex items-start gap-3 p-3 rounded-lg border cursor-pointer transition-colors ${
|
||||
checked ? 'border-blue-500 bg-blue-950/40' : 'border-gray-700 hover:border-gray-500'
|
||||
}`}>
|
||||
<input type="checkbox" className="mt-0.5 accent-blue-500" checked={checked} onChange={() => toggle(svc.key)} />
|
||||
<div>
|
||||
<div className="text-sm font-medium text-white">{svc.label}</div>
|
||||
<div className="text-xs text-gray-400 mt-0.5">{svc.description}</div>
|
||||
</div>
|
||||
</label>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
<div>
|
||||
<p className="text-xs font-medium text-gray-500 uppercase tracking-wider mb-2">Always enabled</p>
|
||||
<div className="space-y-1.5">
|
||||
{ALWAYS_ON_SERVICES.map(svc => (
|
||||
<div key={svc.key} className="flex items-center gap-3 p-3 rounded-lg border border-gray-800 bg-gray-900/40 opacity-60">
|
||||
<input type="checkbox" checked readOnly disabled className="mt-0 accent-blue-500" />
|
||||
<span className="text-sm text-gray-400">{svc.label}</span>
|
||||
<span className="ml-auto text-xs text-gray-600">always enabled</span>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
<NavButtons onBack={onBack} onNext={onNext} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ── 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 (
|
||||
<div>
|
||||
<StepHeader step={5} title="Review and finish"
|
||||
<StepHeader step={4} title="Review and finish"
|
||||
description="Check your choices. You can go back to change anything." />
|
||||
<div className="bg-gray-800/50 border border-gray-700 rounded-lg px-4 py-1 mb-2">
|
||||
<ReviewRow label="Admin password" value="••••••••••••" />
|
||||
<ReviewRow label="Domain" value={domainDisplay} />
|
||||
<ReviewRow label="Provider" value={providerLabel} />
|
||||
<ReviewRow label="Timezone" value={timezone} />
|
||||
<ReviewRow label="Optional services" value={serviceLabels} />
|
||||
</div>
|
||||
{submitError && (
|
||||
<div className="mt-4 p-3 bg-red-950/50 border border-red-700 rounded-lg flex items-start gap-2">
|
||||
@@ -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() {
|
||||
<Step3Timezone value={timezone} onChange={setTimezone} onNext={goNext} onBack={goBack} />
|
||||
)}
|
||||
{step === 4 && (
|
||||
<Step4Services selected={services} onChange={setServices} onNext={goNext} onBack={goBack} />
|
||||
)}
|
||||
{step === 5 && (
|
||||
<Step5Review
|
||||
<Step4Review
|
||||
domainType={domainType} domainName={domainName}
|
||||
services={services} timezone={timezone}
|
||||
timezone={timezone}
|
||||
onBack={goBack} onSubmit={handleSubmit}
|
||||
submitting={submitting} submitError={submitError}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user