mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-12 15:53:46 -04:00
fix(email): keep saved provider on failed switch, fall back to SMTP sender for Resend
- Restore the saved SMTP/Resend flags when a provider switch fails validation, so a later event toggle cannot save a half-configured Resend (team and instance pages). - Resend uses smtp_from_* when resend_from_* is blank, as before the Resend sender fields. - Require the Resend sender only when Resend is enabled. - Replace wire:confirm in the SMTP server picker with x-modal-confirmation.
This commit is contained in:
@@ -244,10 +244,12 @@ class Email extends Component
|
||||
try {
|
||||
$this->resetErrorBag();
|
||||
if (! $this->useInstanceEmailSettings) {
|
||||
$this->validateEnabledEmailProvider();
|
||||
$this->enableSelectedEmailProvider();
|
||||
}
|
||||
$this->saveModel();
|
||||
} catch (\Throwable $e) {
|
||||
$this->restoreSavedEmailProvider();
|
||||
|
||||
return handleError($e, $this);
|
||||
}
|
||||
}
|
||||
@@ -258,11 +260,8 @@ class Email extends Component
|
||||
*/
|
||||
public function selectEmailProvider()
|
||||
{
|
||||
$this->smtpEnabled = $this->emailProvider === 'smtp';
|
||||
$this->resendEnabled = $this->emailProvider === 'resend';
|
||||
|
||||
try {
|
||||
$this->validateEnabledEmailProvider();
|
||||
$this->enableSelectedEmailProvider();
|
||||
} catch (ValidationException) {
|
||||
// Not complete yet: show the fields of the provider and the "Not active yet" notice, without an error.
|
||||
$this->resetErrorBag();
|
||||
@@ -282,6 +281,30 @@ class Email extends Component
|
||||
return enabled_email_provider($this->settings) ?? 'none';
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn on the provider selected in the provider list. When its settings are not valid, the saved
|
||||
* provider stays on, so that a later save of another field cannot turn off the working provider.
|
||||
*/
|
||||
private function enableSelectedEmailProvider(): void
|
||||
{
|
||||
$this->smtpEnabled = $this->emailProvider === 'smtp';
|
||||
$this->resendEnabled = $this->emailProvider === 'resend';
|
||||
|
||||
try {
|
||||
$this->validateEnabledEmailProvider();
|
||||
} catch (ValidationException $e) {
|
||||
$this->restoreSavedEmailProvider();
|
||||
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
|
||||
private function restoreSavedEmailProvider(): void
|
||||
{
|
||||
$this->smtpEnabled = (bool) $this->settings->getOriginal('smtp_enabled');
|
||||
$this->resendEnabled = (bool) $this->settings->getOriginal('resend_enabled');
|
||||
}
|
||||
|
||||
private function validateEnabledEmailProvider(): void
|
||||
{
|
||||
match (true) {
|
||||
@@ -483,8 +506,8 @@ class Email extends Component
|
||||
$this->validate([
|
||||
'resendEnabled' => 'boolean',
|
||||
'resendApiKey' => $this->resendEnabled ? 'required|string' : 'nullable|string',
|
||||
'resendFromAddress' => 'required|email',
|
||||
'resendFromName' => 'required|string',
|
||||
'resendFromAddress' => $this->resendEnabled ? 'required|email' : 'nullable|email',
|
||||
'resendFromName' => $this->resendEnabled ? 'required|string' : 'nullable|string',
|
||||
], [
|
||||
'resendApiKey.required' => 'Resend API Key is required.',
|
||||
'resendFromAddress.required' => 'From Address is required.',
|
||||
|
||||
@@ -150,10 +150,12 @@ class SettingsEmail extends Component
|
||||
try {
|
||||
$this->authorize('update', $this->settings);
|
||||
$this->resetErrorBag();
|
||||
$this->validateEnabledEmailProvider();
|
||||
$this->enableSelectedEmailProvider();
|
||||
$this->syncData(true);
|
||||
$this->dispatch('success', 'Transactional email settings updated.');
|
||||
} catch (\Throwable $e) {
|
||||
$this->restoreSavedEmailProvider();
|
||||
|
||||
return handleError($e, $this);
|
||||
}
|
||||
}
|
||||
@@ -164,11 +166,8 @@ class SettingsEmail extends Component
|
||||
*/
|
||||
public function selectEmailProvider()
|
||||
{
|
||||
$this->smtpEnabled = $this->emailProvider === 'smtp';
|
||||
$this->resendEnabled = $this->emailProvider === 'resend';
|
||||
|
||||
try {
|
||||
$this->validateEnabledEmailProvider();
|
||||
$this->enableSelectedEmailProvider();
|
||||
} catch (ValidationException) {
|
||||
// Not complete yet: show the fields of the provider and the "Not active yet" notice, without an error.
|
||||
$this->resetErrorBag();
|
||||
@@ -179,6 +178,30 @@ class SettingsEmail extends Component
|
||||
return $this->submit();
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn on the provider selected in the provider list. When its settings are not valid, the saved
|
||||
* provider stays on, so that a later save of another field cannot turn off the working provider.
|
||||
*/
|
||||
private function enableSelectedEmailProvider(): void
|
||||
{
|
||||
$this->smtpEnabled = $this->emailProvider === 'smtp';
|
||||
$this->resendEnabled = $this->emailProvider === 'resend';
|
||||
|
||||
try {
|
||||
$this->validateEnabledEmailProvider();
|
||||
} catch (ValidationException $e) {
|
||||
$this->restoreSavedEmailProvider();
|
||||
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
|
||||
private function restoreSavedEmailProvider(): void
|
||||
{
|
||||
$this->smtpEnabled = (bool) $this->settings->getOriginal('smtp_enabled');
|
||||
$this->resendEnabled = (bool) $this->settings->getOriginal('resend_enabled');
|
||||
}
|
||||
|
||||
private function validateEnabledEmailProvider(): void
|
||||
{
|
||||
match (true) {
|
||||
@@ -352,8 +375,8 @@ class SettingsEmail extends Component
|
||||
$this->validate([
|
||||
'resendEnabled' => 'boolean',
|
||||
'resendApiKey' => $this->resendEnabled ? 'required|string' : 'nullable|string',
|
||||
'resendFromAddress' => 'required|email',
|
||||
'resendFromName' => 'required|string',
|
||||
'resendFromAddress' => $this->resendEnabled ? 'required|email' : 'nullable|email',
|
||||
'resendFromName' => $this->resendEnabled ? 'required|string' : 'nullable|string',
|
||||
], [
|
||||
'resendApiKey.required' => 'Resend API Key is required.',
|
||||
'resendFromAddress.required' => 'From Address is required.',
|
||||
|
||||
@@ -52,24 +52,29 @@ function enabled_email_provider(?object $settings): ?string
|
||||
function mail_from_identity(object $settings, bool $asNotification = false): array
|
||||
{
|
||||
// Each provider has its own sender, because each one sends from its own verified domain.
|
||||
$prefix = data_get($settings, 'resend_enabled') ? 'resend' : 'smtp';
|
||||
// Resend uses the SMTP sender when it has none, as before the Resend sender fields existed.
|
||||
$usesResend = (bool) data_get($settings, 'resend_enabled');
|
||||
$providerSender = static function (string $field) use ($settings, $usesResend): string {
|
||||
$value = trim((string) ($usesResend ? data_get($settings, "resend_{$field}") : ''));
|
||||
|
||||
return $value !== '' ? $value : trim((string) data_get($settings, "smtp_{$field}"));
|
||||
};
|
||||
|
||||
if ($asNotification && filled(data_get($settings, 'notification_from_address'))) {
|
||||
$address = (string) data_get($settings, 'notification_from_address');
|
||||
$name = trim((string) data_get($settings, 'notification_from_name', ''));
|
||||
} else {
|
||||
$address = data_get($settings, "{$prefix}_from_address");
|
||||
$address = $providerSender('from_address');
|
||||
|
||||
if (blank($address)) {
|
||||
if ($address === '') {
|
||||
throw new InvalidArgumentException('Transactional email sender address is not configured.');
|
||||
}
|
||||
|
||||
$address = (string) $address;
|
||||
$name = '';
|
||||
}
|
||||
|
||||
if ($name === '') {
|
||||
$name = trim((string) data_get($settings, "{$prefix}_from_name", ''));
|
||||
$name = $providerSender('from_name');
|
||||
}
|
||||
|
||||
return [
|
||||
|
||||
@@ -38,12 +38,20 @@
|
||||
</form>
|
||||
</x-modal-input>
|
||||
@if (count($settings->savedSmtpServers()) > 0)
|
||||
<button type="button" wire:click="deleteSmtpServer"
|
||||
class="icon-button mb-0.5 text-red-500 hover:text-red-600 dark:text-red-400 dark:hover:text-red-300"
|
||||
title="Delete server" aria-label="Delete SMTP server {{ $settings->smtpServerName() }}"
|
||||
wire:confirm="Delete the SMTP server {{ $settings->smtpServerName() }}? Coolify then uses the next server in the list.">
|
||||
<x-reicon name="trash" class="size-3.5" />
|
||||
</button>
|
||||
{{-- Keyed by the server name, so that the ignored modal shows the server in use after a switch. --}}
|
||||
<x-modal-confirmation wire:key="delete-smtp-server-{{ md5($settings->smtpServerName()) }}"
|
||||
title="Delete SMTP server?" submitAction="deleteSmtpServer"
|
||||
:actions="['The SMTP server ' . $settings->smtpServerName() . ' is deleted. Coolify then uses the next server in the list.']"
|
||||
:confirmWithText="false" :confirmWithPassword="false"
|
||||
step2ButtonText="Delete server">
|
||||
<x-slot:trigger>
|
||||
<button type="button"
|
||||
class="icon-button mb-0.5 text-red-500 hover:text-red-600 dark:text-red-400 dark:hover:text-red-300"
|
||||
title="Delete server" aria-label="Delete SMTP server {{ $settings->smtpServerName() }}">
|
||||
<x-reicon name="trash" class="size-3.5" />
|
||||
</button>
|
||||
</x-slot:trigger>
|
||||
</x-modal-confirmation>
|
||||
@endif
|
||||
<x-modal-input title="Add SMTP server">
|
||||
<x-slot:content>
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Notifications\Email as NotificationEmail;
|
||||
use App\Livewire\SettingsEmail;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Once;
|
||||
use Livewire\Livewire;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
function workingSmtpSettings(): array
|
||||
{
|
||||
return [
|
||||
'smtp_enabled' => true,
|
||||
'smtp_host' => 'smtp.example.com',
|
||||
'smtp_port' => '587',
|
||||
'smtp_encryption' => 'starttls',
|
||||
'smtp_from_address' => 'alerts@example.com',
|
||||
'smtp_from_name' => 'Coolify',
|
||||
'resend_enabled' => false,
|
||||
'resend_api_key' => null,
|
||||
];
|
||||
}
|
||||
|
||||
beforeEach(function () {
|
||||
$this->rootTeam = Team::find(0) ?? Team::factory()->create(['id' => 0]);
|
||||
InstanceSettings::forceCreate(['id' => 0, ...workingSmtpSettings()]);
|
||||
Once::flush();
|
||||
});
|
||||
|
||||
test('an unsaved resend selection is not saved by a later team notification event toggle', function () {
|
||||
$team = Team::factory()->create();
|
||||
$user = User::factory()->create();
|
||||
$team->members()->attach($user->id, ['role' => 'admin']);
|
||||
$team->emailNotificationSettings->update([...workingSmtpSettings(), 'use_instance_email_settings' => false]);
|
||||
|
||||
$this->actingAs($user);
|
||||
session(['currentTeam' => $team]);
|
||||
|
||||
Livewire::test(NotificationEmail::class)
|
||||
->set('emailProvider', 'resend')
|
||||
->call('selectEmailProvider')
|
||||
->assertSet('emailProvider', 'resend')
|
||||
->call('toggleEvent', 'deploymentSuccessEmailNotifications')
|
||||
->call('instantSaveDeploymentCommitDetails');
|
||||
|
||||
$settings = $team->emailNotificationSettings->fresh();
|
||||
|
||||
expect($settings->smtp_enabled)->toBeTrue()
|
||||
->and($settings->resend_enabled)->toBeFalse()
|
||||
->and($settings->deployment_success_email_notifications)->toBeTrue();
|
||||
});
|
||||
|
||||
test('an unsaved resend selection is not saved by a later instance provider toggle', function () {
|
||||
$user = User::factory()->create();
|
||||
$this->rootTeam->members()->attach($user->id, ['role' => 'admin']);
|
||||
|
||||
$this->actingAs($user);
|
||||
session(['currentTeam' => ['id' => 0]]);
|
||||
|
||||
Livewire::test(SettingsEmail::class)
|
||||
->set('emailProvider', 'resend')
|
||||
->call('selectEmailProvider')
|
||||
->call('toggleResend');
|
||||
|
||||
$settings = InstanceSettings::find(0);
|
||||
|
||||
expect($settings->smtp_enabled)->toBeTrue()
|
||||
->and($settings->resend_enabled)->toBeFalse();
|
||||
});
|
||||
|
||||
test('a completed resend selection is saved on submit', function () {
|
||||
$team = Team::factory()->create();
|
||||
$user = User::factory()->create();
|
||||
$team->members()->attach($user->id, ['role' => 'admin']);
|
||||
$team->emailNotificationSettings->update([...workingSmtpSettings(), 'use_instance_email_settings' => false]);
|
||||
|
||||
$this->actingAs($user);
|
||||
session(['currentTeam' => $team]);
|
||||
|
||||
Livewire::test(NotificationEmail::class)
|
||||
->set('emailProvider', 'resend')
|
||||
->call('selectEmailProvider')
|
||||
->set('resendApiKey', 're_test_key')
|
||||
->set('resendFromAddress', 'resend@example.com')
|
||||
->set('resendFromName', 'Coolify')
|
||||
->call('submit')
|
||||
->assertHasNoErrors();
|
||||
|
||||
$settings = $team->emailNotificationSettings->fresh();
|
||||
|
||||
expect($settings->resend_enabled)->toBeTrue()
|
||||
->and($settings->smtp_enabled)->toBeFalse()
|
||||
->and($settings->resend_from_address)->toBe('resend@example.com');
|
||||
});
|
||||
|
||||
test('turning team resend off does not require a resend sender', function () {
|
||||
$team = Team::factory()->create();
|
||||
$user = User::factory()->create();
|
||||
$team->members()->attach($user->id, ['role' => 'admin']);
|
||||
$team->emailNotificationSettings->update([
|
||||
'use_instance_email_settings' => false,
|
||||
'resend_enabled' => true,
|
||||
'resend_api_key' => 're_test_key',
|
||||
'smtp_from_address' => 'alerts@example.com',
|
||||
]);
|
||||
|
||||
$this->actingAs($user);
|
||||
session(['currentTeam' => $team]);
|
||||
|
||||
Livewire::test(NotificationEmail::class)
|
||||
->set('resendEnabled', false)
|
||||
->call('submitResend')
|
||||
->assertHasNoErrors()
|
||||
->assertNotDispatched('error');
|
||||
|
||||
expect($team->emailNotificationSettings->fresh()->resend_enabled)->toBeFalse();
|
||||
});
|
||||
@@ -150,8 +150,8 @@ test('resend settings save is audited without the api key', function () {
|
||||
Livewire::test(SettingsEmail::class)
|
||||
->set('resendEnabled', true)
|
||||
->set('resendApiKey', 're_secret_api_key')
|
||||
->set('smtpFromAddress', 'from@example.com')
|
||||
->set('smtpFromName', 'Coolify')
|
||||
->set('resendFromAddress', 'from@example.com')
|
||||
->set('resendFromName', 'Coolify')
|
||||
->call('submitResend');
|
||||
|
||||
$event = instanceSettingsAuditEvents('ui.settings.email.updated')->sole();
|
||||
|
||||
@@ -54,8 +54,8 @@ test('enabling Resend disables SMTP in storage', function () {
|
||||
|
||||
Livewire::test(SettingsEmail::class)
|
||||
->set('resendApiKey', 're_test_key')
|
||||
->set('smtpFromAddress', 'from@example.com')
|
||||
->set('smtpFromName', 'Coolify')
|
||||
->set('resendFromAddress', 'from@example.com')
|
||||
->set('resendFromName', 'Coolify')
|
||||
->call('toggleResend');
|
||||
|
||||
$this->settings->refresh();
|
||||
|
||||
@@ -41,7 +41,7 @@ test('send test is available when transactional email is enabled', function () {
|
||||
->assertSee('Send test');
|
||||
});
|
||||
|
||||
test('send test is not rendered when transactional email is disabled', function () {
|
||||
test('send test is disabled when transactional email is disabled', function () {
|
||||
$rootTeam = Team::find(0) ?? Team::factory()->create(['id' => 0]);
|
||||
InstanceSettings::forceCreate([
|
||||
'id' => 0,
|
||||
@@ -58,5 +58,6 @@ test('send test is not rendered when transactional email is disabled', function
|
||||
|
||||
Livewire::test(SettingsEmail::class)
|
||||
->assertOk()
|
||||
->assertDontSee('Send test');
|
||||
->assertSee('Save an email provider first.')
|
||||
->assertDontSee('Send Test Email');
|
||||
});
|
||||
|
||||
@@ -110,8 +110,7 @@ test('enabling smtp delivery does not require a resend api key when resend is di
|
||||
|
||||
Livewire::test(SettingsEmail::class)
|
||||
->fill(smtpSetupPayload())
|
||||
->set('smtpEnabled', true)
|
||||
->call('instantSaveSmtp')
|
||||
->call('toggleSmtp')
|
||||
->assertHasNoErrors()
|
||||
->assertNotDispatched('error');
|
||||
|
||||
|
||||
@@ -85,3 +85,35 @@ it('rejects enabled email settings without a from address', function () {
|
||||
'smtp_from_name' => 'Coolify',
|
||||
]);
|
||||
})->throws(InvalidArgumentException::class, 'Transactional email sender address is not configured.');
|
||||
|
||||
it('uses the resend sender when resend is enabled', function () {
|
||||
$identity = mail_from_identity((object) [
|
||||
'resend_enabled' => true,
|
||||
'resend_from_address' => 'resend@example.com',
|
||||
'resend_from_name' => 'Resend Sender',
|
||||
'smtp_from_address' => 'smtp@example.com',
|
||||
'smtp_from_name' => 'SMTP Sender',
|
||||
]);
|
||||
|
||||
expect($identity)->toBe(['address' => 'resend@example.com', 'name' => 'Resend Sender']);
|
||||
});
|
||||
|
||||
it('falls back to the smtp sender when resend is enabled without a resend sender', function () {
|
||||
$identity = mail_from_identity((object) [
|
||||
'resend_enabled' => true,
|
||||
'resend_from_address' => null,
|
||||
'resend_from_name' => '',
|
||||
'smtp_from_address' => 'smtp@example.com',
|
||||
'smtp_from_name' => 'SMTP Sender',
|
||||
]);
|
||||
|
||||
expect($identity)->toBe(['address' => 'smtp@example.com', 'name' => 'SMTP Sender']);
|
||||
});
|
||||
|
||||
it('rejects resend settings without a resend or smtp from address', function () {
|
||||
mail_from_identity((object) [
|
||||
'resend_enabled' => true,
|
||||
'resend_from_address' => null,
|
||||
'smtp_from_address' => null,
|
||||
]);
|
||||
})->throws(InvalidArgumentException::class, 'Transactional email sender address is not configured.');
|
||||
|
||||
Reference in New Issue
Block a user