diff --git a/src/Config/GeneralConfig.php b/src/Config/GeneralConfig.php index ffa9bd9db30..d308f86e0c5 100644 --- a/src/Config/GeneralConfig.php +++ b/src/Config/GeneralConfig.php @@ -6025,10 +6025,8 @@ public function tempAssetUploadFs(?string $value): self } /** - * Configures Craft to send all system emails to either a single email address or an array of email addresses - * for testing purposes. - * - * The timezone of the site. If set, it will take precedence over the Timezone setting in Settings → General. + * The timezone of the site. If set, it will take precedence over the Timezone setting in Settings → General + * (stored in project config). * * This can be set to one of PHP’s [supported timezones](https://php.net/manual/en/timezones.php). * diff --git a/src/Console/Commands/Install/InstallCommand.php b/src/Console/Commands/Install/InstallCommand.php index c1d54488d46..bdb46cbcf72 100644 --- a/src/Console/Commands/Install/InstallCommand.php +++ b/src/Console/Commands/Install/InstallCommand.php @@ -7,14 +7,17 @@ use CraftCms\Cms\Cms; use CraftCms\Cms\Config\GeneralConfig; use CraftCms\Cms\Console\CraftCommand; +use CraftCms\Cms\Cp\SelectOptions; use CraftCms\Cms\Database\Migrations\Install; use CraftCms\Cms\Database\Migrator; use CraftCms\Cms\Site\Concerns\SiteDefaults; use CraftCms\Cms\Site\Data\Site; use CraftCms\Cms\Support\Env; use CraftCms\Cms\Translation\I18N; +use CraftCms\Cms\Validation\Rules\EnvValueRule; use Illuminate\Console\Command; use Illuminate\Support\Facades\DB; +use Illuminate\Validation\Rule; use Illuminate\Validation\Rules\Password; use Laravel\Prompts\Support\Logger; use Override; @@ -43,6 +46,7 @@ class InstallCommand extends Command {--siteName= : The default site name for the first site to create during install.} {--siteUrl= : The default site URL for the first site to create during install.} {--language= : The default language for the first site to create during install.} + {--timezone= : The app’s default timezone, typically configured in Settings → General.} '; #[Override] @@ -139,6 +143,23 @@ public function handle( return null; } ), 'language') + ->addIf(! $this->option('timezone'), function () { + $timezoneBaseOptions = array_column(SelectOptions::getTimeZoneOptions(), 'value'); + $timezoneEnvOptions = array_column(SelectOptions::getEnvOptions($timezoneBaseOptions)[0]['options'] ?? [], 'value'); + + return suggest( + label: 'What timezone should the application use?', + options: [ + ...$timezoneBaseOptions, + ...$timezoneEnvOptions, + ], + default: date_default_timezone_get(), + required: true, + validate: [new EnvValueRule([Rule::in($timezoneBaseOptions)])], + hint: 'Type $ for environment variables containing valid timezones.', + info: Env::parse(...), + ); + }, 'timezone') ->submit(); $username = $this->option('username') ?? $responses['username']; @@ -147,6 +168,7 @@ public function handle( $siteName = $this->option('siteName') ?? $responses['siteName']; $siteUrl = $this->option('siteUrl') ?? $responses['siteUrl']; $language = $this->option('language') ?? $responses['language']; + $timezone = $this->option('timezone') ?? $responses['timezone']; if ($generalConfig->useEmailAsUsername) { $username = $email; @@ -184,6 +206,7 @@ public function handle( password: $password, email: $email, site: $site, + timezone: $timezone, ), 'up'); $migrator->getRepository()->log('Install', 1); diff --git a/src/Database/Migrations/Install.php b/src/Database/Migrations/Install.php index c7b2ba70e7a..50d124f8f52 100644 --- a/src/Database/Migrations/Install.php +++ b/src/Database/Migrations/Install.php @@ -53,6 +53,7 @@ public function __construct( public ?string $password = null, public ?string $email = null, public ?Site $site = null, + public ?string $timezone = null, public bool $applyProjectConfigYaml = true, ) { parent::__construct(); @@ -1415,7 +1416,7 @@ private function _generateInitialConfig(): array 'name' => $this->site->getName(), 'live' => true, 'schemaVersion' => Cms::SCHEMA_VERSION, - 'timeZone' => 'America/Los_Angeles', + 'timeZone' => $this->timezone ?? 'America/Los_Angeles', ], 'users' => [ 'requireEmailVerification' => true, diff --git a/src/Http/Controllers/InstallController.php b/src/Http/Controllers/InstallController.php index 40ad2acde5d..ca2b3778449 100644 --- a/src/Http/Controllers/InstallController.php +++ b/src/Http/Controllers/InstallController.php @@ -17,7 +17,9 @@ use CraftCms\Cms\Support\Facades\I18N; use CraftCms\Cms\Support\Str; use CraftCms\Cms\Support\Url; +use CraftCms\Cms\Validation\Rules\EnvValueRule; use CraftCms\Cms\Validation\Rules\LanguageRule; +use CraftCms\Cms\Validation\Rules\TimezoneRule; use Illuminate\Database\SQLiteDatabaseDoesNotExistException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -67,32 +69,33 @@ public function index(GeneralConfig $generalConfig): \Inertia\Response } } - // Grab the license text - $licensePath = Aliases::get('@craftcms/LICENSE.md'); - $license = file_get_contents($licensePath); - $licenseHtml = Str::markdown($license); - // Guess the site name based on the server name $defaultSystemName = $this->defaultSiteName(); $defaultSiteUrl = $this->defaultSiteUrl(); $defaultSiteLanguage = $this->defaultSiteLanguage(); - $locales = I18N::getAllLocales(); $dbConfig = DB::getConfig(); $postCpLoginRedirect = Cms::config()->postCpLoginRedirect; - $localeOptions = collect($locales) - ->map(fn ($locale) => [ + return Inertia::render('Install', [ + 'showDbScreen' => $showDbScreen, + 'postCpLoginRedirect' => $postCpLoginRedirect, + 'licenseHtml' => Inertia::defer(function () { + $licensePath = Aliases::get('@craftcms/LICENSE.md'); + $license = file_get_contents($licensePath); + + return Str::markdown($license); + }), + 'localeOptions' => Inertia::defer(fn () => I18N::getAllLocales()->map(fn ($locale) => [ 'id' => $locale->id, 'value' => $locale->id, 'label' => $locale->getDisplayName(app()->getLocale()), 'selected' => $locale->id === $defaultSiteLanguage, - ]); + ])), + 'timezone' => Inertia::defer(function () { + $timezoneOptions = SelectOptions::getTimeZoneOptions(); - return Inertia::render('Install', [ - 'showDbScreen' => $showDbScreen, - 'postCpLoginRedirect' => $postCpLoginRedirect, - 'licenseHtml' => Inertia::defer(fn () => $licenseHtml), - 'localeOptions' => Inertia::defer(fn () => $localeOptions), + return array_merge($timezoneOptions, SelectOptions::getEnvOptions(array_column($timezoneOptions, 'value'))); + }), 'baseUrlSuggestions' => SelectOptions::getEnvSuggestions(true, fn ($value) => Str::isUrl($value)), 'defaultSystemName' => $defaultSystemName, 'defaultSiteUrl' => $defaultSiteUrl, @@ -148,14 +151,11 @@ public function validateSite(Request $request): Response { $request->validate([ 'name' => ['required', 'string', 'max:255'], - 'baseUrl' => ['nullable', 'string', 'max:255'], + 'baseUrl' => [new EnvValueRule(['nullable', 'url', 'max:255'])], 'language' => ['required', 'string', 'max:255', new LanguageRule(onlySiteLanguages: false)], + 'timezone' => [new EnvValueRule([new TimezoneRule])], ]); - $baseUrl = Env::parse($request->input('baseUrl')); - - Validator::validate(compact('baseUrl'), ['baseUrl' => 'url']); - return new JsonResponse; } @@ -225,6 +225,7 @@ public function install(Request $request, Migrator $migrator, LaravelMigrations password: $request->input('account.password'), email: $email, site: $site, + timezone: $request->input('site.timezone'), )->silent(); // Run the install migration diff --git a/src/Http/Controllers/Settings/GeneralSettingsController.php b/src/Http/Controllers/Settings/GeneralSettingsController.php index f0ca1e68c57..e4851f2ccbb 100644 --- a/src/Http/Controllers/Settings/GeneralSettingsController.php +++ b/src/Http/Controllers/Settings/GeneralSettingsController.php @@ -8,11 +8,10 @@ use CraftCms\Cms\Http\RespondsWithFlash; use CraftCms\Cms\Http\Responses\CpScreenResponse; use CraftCms\Cms\ProjectConfig\ProjectConfig; -use CraftCms\Cms\Support\Env; use CraftCms\Cms\Support\Url; +use CraftCms\Cms\Validation\Rules\EnvValueRule; use CraftCms\Cms\Validation\Rules\TimezoneRule; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Validator; use Symfony\Component\HttpFoundation\Response; use function CraftCms\Cms\t; @@ -27,6 +26,9 @@ public function __construct( public function index(): CpScreenResponse { + $timezoneOptions = SelectOptions::getTimeZoneOptions(); + $timezoneOptions = array_merge($timezoneOptions, SelectOptions::getEnvOptions(array_column($timezoneOptions, 'value'))); + return new CpScreenResponse() ->title(t('General Settings')) ->crumbs([ @@ -37,47 +39,25 @@ public function index(): CpScreenResponse ->inertiaPage('SettingsGeneralPage', [ 'system' => $this->projectConfig->get('system') ?? [], 'nameSuggestions' => SelectOptions::getEnvSuggestions(), - 'timezoneOptions' => [ - ...SelectOptions::getTimeZoneOptions(), - ...SelectOptions::getEnvOptions(), - ], + 'timezoneOptions' => $timezoneOptions, 'systemStatusOptions' => SelectOptions::getBooleanEnvOptions(), ]); } public function store(Request $request): Response { - $resolvedValues = []; - - $envAllowedKeys = ['name', 'live', 'timeZone']; - $booleans = ['live']; - foreach ($request->all() as $key => $value) { - if (in_array($key, $envAllowedKeys) && is_string($value) && str_starts_with($value, '$')) { - $resolvedValues[$key] = in_array($key, $booleans) ? Env::parseBoolean($value) : Env::parse($value); - } else { - $resolvedValues[$key] = $value; - } - } - - /** - * We want to validate against the resolved values, but we'll store what the user provided - */ - Validator::make($resolvedValues, [ - 'name' => ['required', 'string'], - 'live' => ['required', 'boolean'], + $settings = $request->validate([ + 'name' => [new EnvValueRule(['required', 'string'])], + 'live' => [new EnvValueRule(['required', 'boolean'])], 'retryDuration' => ['nullable', 'integer'], - 'timeZone' => ['required', 'string', new TimezoneRule], - ])->validate(); + 'timeZone' => [new EnvValueRule(['required', 'string', new TimezoneRule])], + ]); $systemSettings = $this->projectConfig->get('system') ?? []; - $systemSettings['name'] = $request->input('name'); - $systemSettings['live'] = $request->input('live'); - $systemSettings['retryDuration'] = $request->input('retryDuration') ?: null; - $systemSettings['timeZone'] = $request->input('timeZone'); - - if (! str_starts_with((string) $systemSettings['live'], '$')) { - $systemSettings['live'] = $request->boolean('live'); - } + $systemSettings['name'] = $settings['name']; + $systemSettings['live'] = $settings['live']; + $systemSettings['retryDuration'] = $settings['retryDuration'] ?? null; + $systemSettings['timeZone'] = $settings['timeZone']; $this->projectConfig->set('system', $systemSettings, 'Update system settings.'); diff --git a/src/Validation/Rules/EnvValueRule.php b/src/Validation/Rules/EnvValueRule.php index 8a286cfd0e7..6c76685aef1 100644 --- a/src/Validation/Rules/EnvValueRule.php +++ b/src/Validation/Rules/EnvValueRule.php @@ -71,7 +71,9 @@ public function setData(array $data): static #[Override] public function validate(string $attribute, mixed $value, Closure $fail): void { - $parsedValue = is_string($value) ? Env::parse($value) : $value; + $parsedValue = is_string($value) + ? ($this->isBoolean() ? Env::parseBoolean($value) : Env::parse($value)) + : $value; $data = $this->data; data_set($data, $attribute, $parsedValue); @@ -89,6 +91,17 @@ public function validate(string $attribute, mixed $value, Closure $fail): void } } + private function isBoolean(): bool + { + foreach ($this->rules as $rule) { + if ($rule === 'boolean' || $rule === 'bool') { + return true; + } + } + + return false; + } + private function errorMessage(string $message, mixed $rawValue, mixed $parsedValue): string { if ( diff --git a/tests/Feature/Console/Commands/Install/InstallCommandTest.php b/tests/Feature/Console/Commands/Install/InstallCommandTest.php new file mode 100644 index 00000000000..861c159f86e --- /dev/null +++ b/tests/Feature/Console/Commands/Install/InstallCommandTest.php @@ -0,0 +1,36 @@ +artisan('craft:install') + ->expectsOutputToContain('Craft is already installed!') + ->assertSuccessful(); +}); + +it('exposes the timezone option in the command signature', function () { + $definition = app(InstallCommand::class)->getDefinition(); + + expect($definition->hasOption('timezone'))->toBeTrue() + ->and($definition->getOption('timezone')->getDescription()) + ->toContain('default timezone'); +}); + +it('exposes site, account and language options in the command signature', function () { + $definition = app(InstallCommand::class)->getDefinition(); + + expect($definition->hasOption('email'))->toBeTrue() + ->and($definition->hasOption('username'))->toBeTrue() + ->and($definition->hasOption('password'))->toBeTrue() + ->and($definition->hasOption('siteName'))->toBeTrue() + ->and($definition->hasOption('siteUrl'))->toBeTrue() + ->and($definition->hasOption('language'))->toBeTrue(); +}); + +it('registers the install command aliases', function () { + $command = app(InstallCommand::class); + + expect($command->getAliases())->toEqualCanonicalizing(['craft:install:craft', 'craft:install/craft']); +}); diff --git a/tests/Feature/Http/Controllers/InstallControllerTest.php b/tests/Feature/Http/Controllers/InstallControllerTest.php index 8521950bb5c..71a8e0cf418 100644 --- a/tests/Feature/Http/Controllers/InstallControllerTest.php +++ b/tests/Feature/Http/Controllers/InstallControllerTest.php @@ -17,6 +17,11 @@ Cms::setIsInstalled(false); }); +afterEach(function () { + putenv('INSTALL_TIMEZONE'); + putenv('INSTALL_BASE_URL'); +}); + it('503s if debug is disabled', function () { config()->set('app.debug', false); @@ -30,9 +35,14 @@ ->assertInertia(function (AssertableInertia $page) { $page->component('Install') ->missing('licenseHtml') + ->missing('localeOptions') + ->missing('timezone') ->loadDeferredProps(function (AssertableInertia $reload) { $reload->has('licenseHtml') - ->where('licenseHtml', fn ($html) => str_contains($html, 'Copyright © Pixel & Tonic, Inc.')); + ->where('licenseHtml', fn ($html) => str_contains($html, 'Copyright © Pixel & Tonic, Inc.')) + ->has('localeOptions') + ->has('timezone') + ->where('timezone', fn ($options) => collect($options)->pluck('value')->contains('America/New_York')); }); }) ->assertOk(); @@ -153,8 +163,59 @@ ], 'errors' => ['language'], ], + 'valid timezone' => [ + 'data' => [ + 'name' => 'Craft', + 'baseUrl' => 'http://localhost', + 'language' => 'en', + 'timezone' => 'America/New_York', + ], + 'errors' => [], + ], + 'invalid timezone' => [ + 'data' => [ + 'name' => 'Craft', + 'baseUrl' => 'http://localhost', + 'language' => 'en', + 'timezone' => 'Not/A_Timezone', + ], + 'errors' => ['timezone'], + ], ]); +it('accepts environment variables for the timezone and baseUrl when validating site', function () { + putenv('INSTALL_TIMEZONE=America/New_York'); + putenv('INSTALL_BASE_URL=https://example.test'); + + postJson(action([InstallController::class, 'validateSite']), [ + 'name' => 'Craft', + 'baseUrl' => '$INSTALL_BASE_URL', + 'language' => 'en', + 'timezone' => '$INSTALL_TIMEZONE', + ])->assertOk(); +}); + +it('validates resolved environment variable timezone values when validating site', function () { + putenv('INSTALL_TIMEZONE=Not/A_Timezone'); + + postJson(action([InstallController::class, 'validateSite']), [ + 'name' => 'Craft', + 'baseUrl' => 'http://localhost', + 'language' => 'en', + 'timezone' => '$INSTALL_TIMEZONE', + ])->assertJsonValidationErrors('timezone'); +}); + +it('validates resolved environment variable baseUrl values when validating site', function () { + putenv('INSTALL_BASE_URL=not-an-url'); + + postJson(action([InstallController::class, 'validateSite']), [ + 'name' => 'Craft', + 'baseUrl' => '$INSTALL_BASE_URL', + 'language' => 'en', + ])->assertJsonValidationErrors('baseUrl'); +}); + test('Laravel migration installer can recreate the sessions table', function () { expect(Schema::hasTable('sessions'))->toBeTrue(); diff --git a/tests/Feature/Http/Controllers/Settings/GeneralSettingsControllerTest.php b/tests/Feature/Http/Controllers/Settings/GeneralSettingsControllerTest.php index 4f992cb930f..f3e2346dab5 100644 --- a/tests/Feature/Http/Controllers/Settings/GeneralSettingsControllerTest.php +++ b/tests/Feature/Http/Controllers/Settings/GeneralSettingsControllerTest.php @@ -17,6 +17,13 @@ actingAs(User::find()->one()); }); +afterEach(function () { + putenv('GENERAL_SETTINGS_NAME'); + putenv('GENERAL_SETTINGS_LIVE'); + putenv('GENERAL_SETTINGS_TIMEZONE'); + putenv('GENERAL_SETTINGS_MISSING'); +}); + it('requires authentication', function () { Auth::logout(); @@ -41,12 +48,116 @@ ->assertOk(); }); +it('exposes timezone options on the settings screen', function () { + get(action([GeneralSettingsController::class, 'index'])) + ->assertInertia(fn (AssertableInertia $page) => $page + ->has('timezoneOptions') + ->where('timezoneOptions', fn ($options) => collect($options)->pluck('value')->contains('America/New_York'))) + ->assertOk(); +}); + it('can save settings', function () { post(action([GeneralSettingsController::class, 'store']), [ 'name' => 'A new app name', 'live' => true, + 'retryDuration' => 60, 'timeZone' => 'America/New_York', - ])->assertRedirectBack(); + ])->assertRedirectBack() + ->assertSessionHasNoErrors(); + + expect(ProjectConfig::get('system.name'))->toBe('A new app name') + ->and(ProjectConfig::get('system.live'))->toBe(true) + ->and(ProjectConfig::get('system.retryDuration'))->toBe(60) + ->and(ProjectConfig::get('system.timeZone'))->toBe('America/New_York'); +}); - expect(ProjectConfig::get('system.name'))->toBe('A new app name'); +it('clears retryDuration when not provided', function () { + post(action([GeneralSettingsController::class, 'store']), [ + 'name' => 'App', + 'live' => true, + 'timeZone' => 'America/New_York', + ])->assertRedirectBack() + ->assertSessionHasNoErrors(); + + expect(ProjectConfig::get('system.retryDuration'))->toBeNull(); +}); + +it('validates required fields', function (array $data, array $errors) { + post(action([GeneralSettingsController::class, 'store']), $data) + ->assertSessionHasErrors($errors); +})->with([ + 'all missing' => [ + 'data' => [], + 'errors' => ['name', 'live', 'timeZone'], + ], + 'invalid timezone' => [ + 'data' => [ + 'name' => 'App', + 'live' => true, + 'timeZone' => 'Not/A_Timezone', + ], + 'errors' => ['timeZone'], + ], + 'invalid live boolean' => [ + 'data' => [ + 'name' => 'App', + 'live' => 'definitely', + 'timeZone' => 'America/New_York', + ], + 'errors' => ['live'], + ], + 'invalid retryDuration' => [ + 'data' => [ + 'name' => 'App', + 'live' => true, + 'retryDuration' => 'soon', + 'timeZone' => 'America/New_York', + ], + 'errors' => ['retryDuration'], + ], +]); + +it('can save settings with environment variables', function () { + putenv('GENERAL_SETTINGS_NAME=Env App'); + putenv('GENERAL_SETTINGS_LIVE=true'); + putenv('GENERAL_SETTINGS_TIMEZONE=America/New_York'); + + post(action([GeneralSettingsController::class, 'store']), [ + 'name' => '$GENERAL_SETTINGS_NAME', + 'live' => '$GENERAL_SETTINGS_LIVE', + 'timeZone' => '$GENERAL_SETTINGS_TIMEZONE', + ])->assertRedirectBack() + ->assertSessionHasNoErrors(); + + expect(ProjectConfig::get('system.name'))->toBe('$GENERAL_SETTINGS_NAME') + ->and(ProjectConfig::get('system.live'))->toBe('$GENERAL_SETTINGS_LIVE') + ->and(ProjectConfig::get('system.timeZone'))->toBe('$GENERAL_SETTINGS_TIMEZONE'); +}); + +it('validates resolved environment variable live values', function () { + putenv('GENERAL_SETTINGS_LIVE=maybe'); + + post(action([GeneralSettingsController::class, 'store']), [ + 'name' => 'App', + 'live' => '$GENERAL_SETTINGS_LIVE', + 'timeZone' => 'America/New_York', + ])->assertSessionHasErrors('live'); +}); + +it('validates resolved environment variable timezone values', function () { + putenv('GENERAL_SETTINGS_TIMEZONE=Not/A_Timezone'); + + post(action([GeneralSettingsController::class, 'store']), [ + 'name' => 'App', + 'live' => true, + 'timeZone' => '$GENERAL_SETTINGS_TIMEZONE', + ])->assertSessionHasErrors('timeZone'); +}); + +it('fails required validation for missing environment variables', function () { + post(action([GeneralSettingsController::class, 'store']), [ + 'name' => '$GENERAL_SETTINGS_MISSING', + 'live' => true, + 'timeZone' => 'America/New_York', + ])->assertSessionHasErrors('name'); }); diff --git a/tests/Unit/Validation/Rules/EnvValueRuleTest.php b/tests/Unit/Validation/Rules/EnvValueRuleTest.php index 1f83b3f85a2..5b31d22cbfb 100644 --- a/tests/Unit/Validation/Rules/EnvValueRuleTest.php +++ b/tests/Unit/Validation/Rules/EnvValueRuleTest.php @@ -58,6 +58,7 @@ function makeEnvValueRuleValidator(mixed $value, array $rules) putenv('ENV_VALUE_RULE_EMAIL'); putenv('ENV_VALUE_RULE_MAILER'); putenv('ENV_VALUE_RULE_MISSING'); + putenv('ENV_VALUE_RULE_BOOL'); putenv('PASSWORD'); Aliases::remove('@env-value-rule-email'); }); @@ -134,6 +135,25 @@ function (string $attribute, mixed $value, Closure $fail) { 'sensitive environment variable' => ['$PASSWORD', 'Invalid value.', fn () => putenv('PASSWORD=resolved-value')], ]); +it('parses boolean environment variables for the boolean rule', function (string $envValue, bool $shouldPass) { + putenv("ENV_VALUE_RULE_BOOL={$envValue}"); + + $validator = makeEnvValueRuleValidator('$ENV_VALUE_RULE_BOOL', ['required', 'boolean']); + + expect($validator->passes())->toBe($shouldPass); +})->with([ + 'true' => ['true', true], + 'false' => ['false', true], + 'yes' => ['yes', true], + 'no' => ['no', true], + 'on' => ['on', true], + 'off' => ['off', true], + '1' => ['1', true], + '0' => ['0', true], + 'maybe' => ['maybe', false], + 'empty' => ['', false], +]); + it('works from rulesets without mutating the subject value', function () { putenv('ENV_VALUE_RULE_EMAIL=test@example.com'); $component = makeEnvValueRuleTestComponent('$ENV_VALUE_RULE_EMAIL');