fix: adversarial-review fixes + themed checkboxes
Multi-agent review of the feedback batch (7 confirmed): - [P2] Low-battery sweep excluded a genuine 0% reading (`$pct > 0`) — a dead battery never alerted. Now `< 20` (missing reading still defaults to 100). - [P2] SMTP encryption rule `required|in:,tls,ssl` rejected the "None" option it offered → unencrypted relays uncofigurable. Now `in:,tls,ssl`. - [P2] Avatar preview called temporaryUrl() on any upload → 500 for HEIC/ non-previewable files. Guarded with isPreviewable() + real-time updatedAvatar() validation for a friendly error. - [P3] NotificationService::send() now catches transport errors so one SMTP failure can't abort a whole sweep / automation run. - [P3] Mailer config is now purged (Mail::purge) so long-running Horizon workers pick up SMTP setting changes instead of reusing a cached mailer. - [P3] Alert dedup TTL 1 day → 1 month, so a persistent outage mails once, not daily. - User: themed .checkbox (accent, dark-surface, custom check) replaces the default browser checkbox in SMTP/automation/device modals. +2 tests (0% battery alerts, no-encryption saves). Suite 84 green, 12/12 clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>feat/phase-1-bootstrap
parent
675d04e104
commit
97dc49e577
|
|
@ -41,7 +41,9 @@ class NotificationsSweepCommand extends Command
|
|||
if (! Cache::has($key) && $notifier->notify('device_offline',
|
||||
__('notifications.device_offline_subject', ['name' => $device->name]),
|
||||
__('notifications.device_offline_body', ['name' => $device->name]))) {
|
||||
Cache::put($key, true, now()->addDay());
|
||||
// Flag stays until the condition RESOLVES (cleared below) — one mail per
|
||||
// continuous outage, not one every 24h. The long TTL is just a leak guard.
|
||||
Cache::put($key, true, now()->addMonth());
|
||||
}
|
||||
} else {
|
||||
Cache::forget($key);
|
||||
|
|
@ -58,14 +60,16 @@ class NotificationsSweepCommand extends Command
|
|||
continue;
|
||||
}
|
||||
|
||||
// Missing reading defaults to 100 (no alert); a genuine 0% is a dead battery and MUST
|
||||
// alert — so the threshold is simply < 20 (a real 0 included).
|
||||
$pct = (int) data_get($entity->state, 'state.percent', 100);
|
||||
$key = "notified:battery:{$entity->id}";
|
||||
|
||||
if ($pct > 0 && $pct < 20) {
|
||||
if ($pct < 20) {
|
||||
if (! Cache::has($key) && $notifier->notify('low_battery',
|
||||
__('notifications.low_battery_subject', ['name' => $entity->device->name]),
|
||||
__('notifications.low_battery_body', ['name' => $entity->device->name, 'percent' => $pct]))) {
|
||||
Cache::put($key, true, now()->addDay());
|
||||
Cache::put($key, true, now()->addMonth());
|
||||
}
|
||||
} elseif ($pct >= 25) {
|
||||
Cache::forget($key); // hysteresis so it doesn't flap around the threshold
|
||||
|
|
|
|||
|
|
@ -58,6 +58,12 @@ class ManagePerson extends ModalComponent
|
|||
}
|
||||
}
|
||||
|
||||
/** Validate the picture as soon as it's chosen, so a non-image shows a friendly error. */
|
||||
public function updatedAvatar(): void
|
||||
{
|
||||
$this->validateOnly('avatar', ['avatar' => ['nullable', 'image', 'max:4096']]);
|
||||
}
|
||||
|
||||
public function save()
|
||||
{
|
||||
$this->validate([
|
||||
|
|
|
|||
|
|
@ -64,7 +64,8 @@ class SmtpSetup extends ModalComponent
|
|||
return [
|
||||
'host' => 'required|string|max:190',
|
||||
'port' => 'required|integer|min:1|max:65535',
|
||||
'encryption' => 'required|in:,tls,ssl',
|
||||
// '' (None) is a valid choice — 'required' would reject it, so validate membership only.
|
||||
'encryption' => 'in:,tls,ssl',
|
||||
'username' => 'nullable|string|max:190',
|
||||
'password' => $this->hasStoredPassword ? 'nullable|string|max:190' : 'nullable|string|max:190',
|
||||
'fromEmail' => 'required|email|max:190',
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ class NotificationService
|
|||
return $this->send($subject, $body);
|
||||
}
|
||||
|
||||
/** Send unconditionally (used by the "send test" button). Returns success. */
|
||||
/** Send unconditionally (used by the "send test" button). Returns success, never throws. */
|
||||
public function send(string $subject, string $body): bool
|
||||
{
|
||||
$cfg = $this->config();
|
||||
|
|
@ -43,11 +43,19 @@ class NotificationService
|
|||
|
||||
$this->configureMailer($cfg);
|
||||
|
||||
try {
|
||||
Mail::mailer('homeos_smtp')->raw($body, function ($message) use ($cfg) {
|
||||
$message->to($cfg['to_email'])
|
||||
->subject('[HomeOS] '.$subject)
|
||||
->from($cfg['from_email'] ?: 'homeos@localhost', $cfg['from_name'] ?: 'HomeOS');
|
||||
});
|
||||
} catch (\Throwable $e) {
|
||||
// A transient SMTP failure must not abort the caller (a sweep over many devices, an
|
||||
// automation run). Log and report failure so the loop keeps going.
|
||||
report($e);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
@ -64,5 +72,9 @@ class NotificationService
|
|||
'password' => ($cfg['password'] ?? '') ?: null,
|
||||
'timeout' => 10,
|
||||
]]);
|
||||
|
||||
// MailManager caches the resolved mailer by name; purge it so a long-running worker
|
||||
// (Horizon) picks up config changes instead of reusing the first mailer it built.
|
||||
Mail::purge('homeos_smtp');
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -109,6 +109,29 @@
|
|||
|
||||
.pulse-live { animation: pulse 2.2s ease-in-out infinite; }
|
||||
.reveal { opacity: 0; transform: translateY(10px); animation: reveal .5s cubic-bezier(.2,.7,.3,1) forwards; }
|
||||
|
||||
/* Themed checkbox — the native one looks off on OLED dark; this matches the console. */
|
||||
.checkbox {
|
||||
appearance: none; -webkit-appearance: none;
|
||||
width: 1.05rem; height: 1.05rem; flex-shrink: 0;
|
||||
border-radius: 5px;
|
||||
border: 1px solid var(--color-line);
|
||||
background: var(--color-raised);
|
||||
cursor: pointer; position: relative;
|
||||
transition: background-color .15s, border-color .15s;
|
||||
}
|
||||
.checkbox:hover { border-color: color-mix(in srgb, var(--color-accent) 50%, var(--color-line)); }
|
||||
.checkbox:checked { background: var(--color-accent); border-color: var(--color-accent); }
|
||||
.checkbox:checked::after {
|
||||
content: ""; position: absolute; left: 5px; top: 2px;
|
||||
width: 4px; height: 8px;
|
||||
border: solid var(--color-base); border-width: 0 2px 2px 0;
|
||||
transform: rotate(45deg);
|
||||
}
|
||||
.checkbox:focus-visible {
|
||||
outline: 2px solid color-mix(in srgb, var(--color-accent) 55%, transparent);
|
||||
outline-offset: 1px;
|
||||
}
|
||||
}
|
||||
|
||||
@keyframes pulse { 0%, 100% { opacity: 1; } 50% { opacity: .35; } }
|
||||
|
|
|
|||
|
|
@ -111,7 +111,7 @@
|
|||
@if ($role)
|
||||
<label class="inline-flex items-center gap-1.5 text-[11.5px] text-ink-2 cursor-pointer">
|
||||
<input type="checkbox" wire:change="setInputInvert({{ $entity->id }}, $event.target.checked)" @checked(data_get($role, 'invert'))
|
||||
class="w-3.5 h-3.5 rounded border-line text-accent focus:ring-accent">
|
||||
class="checkbox">
|
||||
{{ __('devices.role_invert') }}
|
||||
</label>
|
||||
@endif
|
||||
|
|
|
|||
|
|
@ -93,7 +93,7 @@
|
|||
</div>
|
||||
|
||||
<label class="flex items-start gap-3 cursor-pointer self-end pb-1">
|
||||
<input wire:model="dryRun" type="checkbox" class="mt-0.5 w-4 h-4 rounded border-line text-accent focus:ring-accent">
|
||||
<input wire:model="dryRun" type="checkbox" class="checkbox mt-0.5">
|
||||
<span class="flex flex-col">
|
||||
<span class="text-[13px] font-semibold text-ink">{{ __('automations.dry_run') }}</span>
|
||||
<span class="text-[11.5px] text-ink-3">{{ __('automations.dry_run_hint') }}</span>
|
||||
|
|
|
|||
|
|
@ -4,7 +4,9 @@
|
|||
{{-- Avatar --}}
|
||||
<div class="flex items-center gap-4">
|
||||
<div class="relative shrink-0">
|
||||
@php $preview = $avatar ? $avatar->temporaryUrl() : $currentAvatarUrl; @endphp
|
||||
{{-- isPreviewable() guards non-previewable uploads (e.g. iPhone HEIC): calling
|
||||
temporaryUrl() on them throws — fall back to the stored/initials avatar. --}}
|
||||
@php $preview = ($avatar && $avatar->isPreviewable()) ? $avatar->temporaryUrl() : $currentAvatarUrl; @endphp
|
||||
@if ($preview)
|
||||
<img src="{{ $preview }}" alt="" class="w-16 h-16 rounded-full object-cover border border-line-soft">
|
||||
@else
|
||||
|
|
|
|||
|
|
@ -59,9 +59,9 @@
|
|||
{{-- Events --}}
|
||||
<fieldset class="flex flex-col gap-2.5 rounded-xl border border-line-soft p-3.5">
|
||||
<legend class="px-1.5 text-[11px] font-bold uppercase tracking-wide text-ink-3">{{ __('addons.smtp.events_title') }}</legend>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evDeviceOffline" class="w-4 h-4 rounded border-line text-accent focus:ring-accent"> {{ __('addons.smtp.ev_device_offline') }}</label>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evLowBattery" class="w-4 h-4 rounded border-line text-accent focus:ring-accent"> {{ __('addons.smtp.ev_low_battery') }}</label>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evAutomation" class="w-4 h-4 rounded border-line text-accent focus:ring-accent"> {{ __('addons.smtp.ev_automation') }}</label>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evDeviceOffline" class="checkbox"> {{ __('addons.smtp.ev_device_offline') }}</label>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evLowBattery" class="checkbox"> {{ __('addons.smtp.ev_low_battery') }}</label>
|
||||
<label class="flex items-center gap-2.5 cursor-pointer text-[13px] text-ink"><input type="checkbox" wire:model="evAutomation" class="checkbox"> {{ __('addons.smtp.ev_automation') }}</label>
|
||||
</fieldset>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -73,6 +73,34 @@ class NotificationTest extends TestCase
|
|||
$this->assertSame('secret', Addon::where('key', 'smtp')->first()->config['password']);
|
||||
}
|
||||
|
||||
public function test_dead_battery_at_zero_percent_alerts(): void
|
||||
{
|
||||
Mail::fake();
|
||||
$this->configureSmtp();
|
||||
|
||||
$device = \App\Models\Device::create(['name' => 'Sensor', 'vendor' => 'Shelly', 'protocol' => 'mqtt', 'status' => 'active', 'demo' => false, 'last_seen_at' => now(), 'config' => ['mqtt_prefix' => 'x']]);
|
||||
$entity = \App\Models\Entity::create(['device_id' => $device->id, 'type' => 'battery', 'key' => 'battery:0']);
|
||||
\App\Models\DeviceState::create(['entity_id' => $entity->id, 'state' => ['percent' => 0]]);
|
||||
|
||||
$this->artisan('notifications:sweep')->assertSuccessful();
|
||||
|
||||
$this->assertTrue(Cache::has("notified:battery:{$entity->id}"));
|
||||
}
|
||||
|
||||
public function test_smtp_setup_allows_no_encryption(): void
|
||||
{
|
||||
$this->actingAs(User::factory()->create());
|
||||
|
||||
Livewire::test(SmtpSetup::class)
|
||||
->set('host', 'localhost')->set('port', '25')->set('encryption', '')
|
||||
->set('fromEmail', 'a@b.com')->set('toEmail', 'c@d.com')
|
||||
->call('save')
|
||||
->assertHasNoErrors()
|
||||
->assertRedirect(route('addons.index'));
|
||||
|
||||
$this->assertSame('', Addon::where('key', 'smtp')->first()->config['encryption']);
|
||||
}
|
||||
|
||||
public function test_sweep_emails_once_per_offline_device(): void
|
||||
{
|
||||
Mail::fake();
|
||||
|
|
|
|||
Loading…
Reference in New Issue