From 920de623a0f234691ed22dae3151b15888b62eb9 Mon Sep 17 00:00:00 2001 From: Matt Edholm Date: Fri, 8 May 2026 14:53:51 -0400 Subject: [PATCH] feat(devices): owner can mark a frame as sold and unlink it pre-emptively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pairs with the new claim-on-takeover checkbox: now the seller can purge their data BEFORE handing the device over, so even if they forget to hold the BOOT button to wipe NVS, the next owner can't accidentally pull their photos. Backend: - DELETE /api/devices/{id}: owner-only (404 for cross-tenant). Revokes image-device approvals, drops history rows, removes the Device row entirely so the MAC is unclaimed. The next poll from that physical frame returns 404 → setup QR for the next owner. - DeviceService::deleteDeviceForOwner extracts the cleanup so the controller stays thin. - Mercure publish on delete sends {id, deleted: true} so any other open PWA tabs splice the row out instantly. Frontend: - Settings sheet (BaseBottomSheet): "Remove this frame" link below Save, in danger red with an explanatory hint about when to use it. - Native window.confirm gate — destructive + irreversible, the weight of native-confirm is honest. (A bespoke modal would be polish.) - useDeviceMercure: handles the {id, deleted: true} sentinel — splices the device out + closes its own EventSource for that topic. - useDevicesStore.removeDevice: DELETE + local store filter. Tests added: - DeviceApiControllerTest: 4 cases — happy-path delete purges everything, 404 cross-tenant, anon redirects to login, and post-delete the device-poll endpoint 404s (fresh-MAC guarantee). - HomeView.test.ts: confirm-yes calls store + closes sheet, confirm-cancel does NOT call removeDevice. - useDeviceMercure.test.ts: deletion sentinel splices the device out and closes the EventSource. Coverage: 99.71% lines / 98.21% methods backend, 98.31% lines frontend. 558 tests total via ddev tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/src/composables/useDeviceMercure.ts | 10 ++- frontend/src/stores/devices.ts | 14 ++- .../test/composables/useDeviceMercure.test.ts | 21 +++++ frontend/src/test/views/HomeView.test.ts | 39 ++++++++ frontend/src/views/HomeView.vue | 69 +++++++++++++++ src/Controller/DeviceApiController.php | 35 ++++++++ src/Service/DeviceService.php | 17 ++++ .../Controller/DeviceApiControllerTest.php | 88 +++++++++++++++++++ 8 files changed, 291 insertions(+), 2 deletions(-) diff --git a/frontend/src/composables/useDeviceMercure.ts b/frontend/src/composables/useDeviceMercure.ts index e6f81da..01e11eb 100644 --- a/frontend/src/composables/useDeviceMercure.ts +++ b/frontend/src/composables/useDeviceMercure.ts @@ -48,7 +48,15 @@ export function useDeviceMercure() { es.onmessage = (event) => { try { - const updated = JSON.parse(event.data) as Device + const payload = JSON.parse(event.data) as Device | { id: number; deleted: true } + // Deletion sentinel: server sends {id, deleted: true} when the + // owner removed the frame. Splice out + close our subscription. + if ('deleted' in payload && payload.deleted === true) { + devices.value = devices.value.filter(d => d.id !== payload.id) + close(payload.id) + return + } + const updated = payload as Device const idx = devices.value.findIndex(d => d.id === updated.id) if (idx !== -1) { // Splice replacement so Vue's reactivity tracks the swap. diff --git a/frontend/src/stores/devices.ts b/frontend/src/stores/devices.ts index bd71483..9b6de93 100644 --- a/frontend/src/stores/devices.ts +++ b/frontend/src/stores/devices.ts @@ -52,6 +52,18 @@ export const useDevicesStore = defineStore('devices', () => { return updated } + /** + * Owner-initiated remove. Hits the API DELETE; on success splices the + * device out of the local list so the UI updates immediately. The + * Mercure subscription will also receive a `{deleted: true}` push for + * any other tabs the user has open. + */ + async function removeDevice(id: number): Promise { + const res = await fetch(`/api/devices/${id}`, { method: 'DELETE' }) + if (!res.ok) throw new Error('Failed to remove device') + devices.value = devices.value.filter(d => d.id !== id) + } + async function unlockImage(deviceId: number): Promise { const res = await fetch(`/api/devices/${deviceId}/lock`, { method: 'DELETE' }) if (!res.ok) throw new Error('Failed to unlock') @@ -61,5 +73,5 @@ export const useDevicesStore = defineStore('devices', () => { return updated } - return { devices, loading, error, fetchDevices, updateDevice, lockImage, unlockImage } + return { devices, loading, error, fetchDevices, updateDevice, removeDevice, lockImage, unlockImage } }) diff --git a/frontend/src/test/composables/useDeviceMercure.test.ts b/frontend/src/test/composables/useDeviceMercure.test.ts index 6e6445f..3ff5c95 100644 --- a/frontend/src/test/composables/useDeviceMercure.test.ts +++ b/frontend/src/test/composables/useDeviceMercure.test.ts @@ -163,6 +163,27 @@ describe('useDeviceMercure', () => { expect(instances).toHaveLength(2) // a fresh connection was opened }) + it('removes the device from the store on a {deleted: true} sentinel', async () => { + const store = useDevicesStore() + store.devices = [makeDevice({ id: 7, name: 'Den' }), makeDevice({ id: 9, name: 'Cabin' })] + + mountWithComposable() + await flushPromises() + expect(instances).toHaveLength(2) + + // Find the EventSource instance subscribed to device 7. + const dev7 = instances.find(i => i.url.includes('%2F7'))! + dev7.onmessage?.(new MessageEvent('message', { + data: JSON.stringify({ id: 7, deleted: true }), + })) + await flushPromises() + + // Device 7 spliced out, 9 still there. The deleted device's EventSource + // is closed too — no point keeping a connection for a vanished device. + expect(store.devices.map(d => d.id)).toEqual([9]) + expect(dev7.close).toHaveBeenCalled() + }) + it('closes EventSources for devices that disappear from the store', async () => { const store = useDevicesStore() store.devices = [makeDevice({ id: 7 }), makeDevice({ id: 9 })] diff --git a/frontend/src/test/views/HomeView.test.ts b/frontend/src/test/views/HomeView.test.ts index 577026c..bc3582a 100644 --- a/frontend/src/test/views/HomeView.test.ts +++ b/frontend/src/test/views/HomeView.test.ts @@ -1028,6 +1028,45 @@ describe('HomeView', () => { })) }) + it('Remove this frame confirms, calls store.removeDevice, and closes the sheet', async () => { + const devicesStore = useDevicesStore() + devicesStore.devices = [makeDevice({ id: 5, name: 'Den' })] + vi.spyOn(devicesStore, 'fetchDevices').mockResolvedValue() + const removeSpy = vi.spyOn(devicesStore, 'removeDevice').mockResolvedValue() + const confirmSpy = vi.fn().mockReturnValue(true) + vi.stubGlobal('confirm', confirmSpy) + + const wrapper = mountView() + await flushPromises() + await wrapper.findComponent({ name: 'FrameCard' }).vm.$emit('edit', 5) + await flushPromises() + + await wrapper.find('.home-view__remove').trigger('click') + await flushPromises() + + expect(confirmSpy).toHaveBeenCalled() + expect(removeSpy).toHaveBeenCalledWith(5) + const sheet = wrapper.findComponent({ name: 'BaseBottomSheet' }) + expect(sheet.props('modelValue')).toBe(false) + }) + + it('Remove this frame cancel does NOT call removeDevice', async () => { + const devicesStore = useDevicesStore() + devicesStore.devices = [makeDevice({ id: 5 })] + vi.spyOn(devicesStore, 'fetchDevices').mockResolvedValue() + const removeSpy = vi.spyOn(devicesStore, 'removeDevice').mockResolvedValue() + vi.stubGlobal('confirm', vi.fn().mockReturnValue(false)) + + const wrapper = mountView() + await flushPromises() + await wrapper.findComponent({ name: 'FrameCard' }).vm.$emit('edit', 5) + await flushPromises() + await wrapper.find('.home-view__remove').trigger('click') + await flushPromises() + + expect(removeSpy).not.toHaveBeenCalled() + }) + it('shows the propagation note in the settings sheet', async () => { const devicesStore = useDevicesStore() devicesStore.devices = [makeDevice({ id: 5 })] diff --git a/frontend/src/views/HomeView.vue b/frontend/src/views/HomeView.vue index f0b8f9f..115a3b7 100644 --- a/frontend/src/views/HomeView.vue +++ b/frontend/src/views/HomeView.vue @@ -223,6 +223,20 @@ > {{ saving ? 'Saving…' : 'Save' }} + + +

+ Use this if you’re selling or giving away the frame. It deletes + this frame from your account and unlinks it from your photos so the + next owner can claim it fresh. +

@@ -481,6 +495,7 @@ const TIMEZONE_GROUPS = [ const sheetOpen = ref(false) const saving = ref(false) +const removing = ref(false) const editingDevice = ref(null) const editName = ref('') const editOrientation = ref('landscape') @@ -637,6 +652,27 @@ function onEdit(deviceId: number) { sheetOpen.value = true } +async function confirmAndRemove() { + if (!editingDevice.value) return + const name = editingDevice.value.name || 'this frame' + // Native confirm — destructive, irreversible, single-user action. A + // bespoke modal would be polish; native is honest about the weight. + const ok = window.confirm( + `Remove "${name}" from your account?\n\n` + + `This deletes the frame's history and unlinks all photos you'd ` + + `approved for it. Use this when selling or giving the frame away.`, + ) + if (!ok) return + + removing.value = true + try { + await devicesStore.removeDevice(editingDevice.value.id) + sheetOpen.value = false + } finally { + removing.value = false + } +} + async function saveSettings() { if (!editingDevice.value) return saving.value = true @@ -972,5 +1008,38 @@ async function saveSettings() { width: 100%; margin-top: var(--space-2); } + + &__remove { + width: 100%; + margin-top: var(--space-5); + padding: var(--space-2) var(--space-3); + min-height: var(--touch-min); + border: 1.5px solid transparent; + border-radius: var(--radius-md); + background: transparent; + color: var(--color-danger, #c0392b); + font-size: var(--text-sm); + font-weight: 600; + cursor: pointer; + transition: all var(--duration-fast); + + &:hover, &:focus-visible { + border-color: var(--color-danger, #c0392b); + outline: none; + } + + &:disabled { + opacity: 0.6; + cursor: not-allowed; + } + } + + &__remove-hint { + margin-top: var(--space-1); + font-size: var(--text-xs); + color: var(--color-text-muted); + line-height: 1.4; + text-align: center; + } } diff --git a/src/Controller/DeviceApiController.php b/src/Controller/DeviceApiController.php index cb4f473..66d8f40 100644 --- a/src/Controller/DeviceApiController.php +++ b/src/Controller/DeviceApiController.php @@ -12,6 +12,7 @@ use App\Enum\Orientation; use App\Enum\RenderStatus; use App\Enum\RotationMode; use App\Service\DeviceSerializer; +use App\Service\DeviceService; use App\Service\MercurePublisher; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -45,6 +46,7 @@ class DeviceApiController extends AbstractController private readonly string $projectDir, private readonly DeviceSerializer $serializer, private readonly MercurePublisher $mercure, + private readonly DeviceService $deviceService, ) {} #[Route('', name: 'api_devices_list', methods: ['GET'])] @@ -193,6 +195,39 @@ class DeviceApiController extends AbstractController return $this->json($payload); } + /** + * Owner-initiated removal — "I sold this frame / I'm not using it + * anymore." Revokes image approvals, drops display history, and removes + * the Device row entirely so the MAC is unclaimed. The next time that + * physical device polls, the server returns 404 and the device shows + * its setup QR for whoever provisions it next. + * + * Pre-emptive purge: if the user runs this BEFORE handing the device + * away, the new owner can't accidentally pull their photos even if the + * seller forgets to hold the BOOT button to wipe NVS first. + */ + #[Route('/{id}', name: 'api_device_delete', methods: ['DELETE'])] + public function delete(int $id, EntityManagerInterface $em): Response + { + /** @var User $user */ + $user = $this->getUser(); + $device = $em->getRepository(Device::class)->findOneBy(['id' => $id, 'user' => $user]); + + if (!$device) { + return $this->json(['error' => 'Device not found'], Response::HTTP_NOT_FOUND); + } + + $deviceId = (int) $device->getId(); + $this->deviceService->deleteDeviceForOwner($device); + + // Tell any subscribed PWA tabs the device is gone so multi-tab + // sessions and Mom's-phone-still-watching stay consistent. The + // payload is a deletion sentinel; the SPA listener splices the row + // out of its store on receipt. + $this->mercure->publishDevice($deviceId, ['id' => $deviceId, 'deleted' => true]); + + return new Response(null, Response::HTTP_NO_CONTENT); + } /** * Serve a PNG preview of the image **currently shown on the frame**, diff --git a/src/Service/DeviceService.php b/src/Service/DeviceService.php index 04abd56..eec0202 100644 --- a/src/Service/DeviceService.php +++ b/src/Service/DeviceService.php @@ -77,6 +77,23 @@ class DeviceService return $device; } + /** + * Owner-initiated delete. Revokes image-device approvals, drops history, + * and removes the Device row entirely so the MAC is fully unclaimed — + * the next provisioning poll from this device will return 404 and the + * frame will display the setup QR for whoever provisions it next. + * + * The user-facing pitch is "I'm selling/giving this away" — by purging + * before handing the device over, the new owner can't accidentally pull + * the seller's photos even if the seller forgets to wipe NVS first. + */ + public function deleteDeviceForOwner(Device $device): void + { + $this->purgeDeviceHistory($device); + $this->em->remove($device); + $this->em->flush(); + } + /** Remove all image approvals and display history for this device. */ private function purgeDeviceHistory(Device $device): void { diff --git a/tests/Functional/Controller/DeviceApiControllerTest.php b/tests/Functional/Controller/DeviceApiControllerTest.php index e77612b..ed8de8e 100644 --- a/tests/Functional/Controller/DeviceApiControllerTest.php +++ b/tests/Functional/Controller/DeviceApiControllerTest.php @@ -576,6 +576,94 @@ class DeviceApiControllerTest extends AppWebTestCase @unlink(preg_replace('/\.bin$/', '.png', $absPath)); } + // ── DELETE /api/devices/{id} (sell/give-away) ──────────────────────── + + public function test_delete_removes_device_and_purges_history_and_approvals(): void + { + $owner = $this->createUser('del@example.com'); + $device = $this->makeDevice('AA:BB:CC:DD:EE:F0', $owner); + $deviceId = $device->getId(); + + // Pre-existing approvals + history we expect the delete to wipe. + $image = $this->makeImage($owner); + $image->approveForDevice($device); + $history = new \App\Entity\DeviceImageHistory($device, $image); + $this->em()->persist($history); + $this->em()->flush(); + + $client = $this->loginAs($owner); + $client->request('DELETE', '/api/devices/' . $deviceId); + + $this->assertResponseStatusCodeSame(204); + + $this->em()->clear(); + $this->assertNull($this->em()->find(\App\Entity\Device::class, $deviceId), 'device row removed'); + + // Approval revoked: image still exists, but no longer approved for the deleted device id. + $reloadedImage = $this->em()->find(\App\Entity\Image::class, $image->getId()); + $approvedIds = array_map( + fn(\App\Entity\Device $d) => $d->getId(), + $reloadedImage->getApprovedDevices()->toArray(), + ); + $this->assertNotContains($deviceId, $approvedIds); + + // History rows for the deleted device cascaded out. + $count = (int) $this->em()->createQueryBuilder() + ->select('COUNT(h.id)') + ->from(\App\Entity\DeviceImageHistory::class, 'h') + ->where('h.device = :id') + ->setParameter('id', $deviceId) + ->getQuery() + ->getSingleScalarResult(); + $this->assertSame(0, $count); + } + + public function test_delete_404_for_other_users_device(): void + { + $owner = $this->createUser('del-own@example.com'); + $other = $this->createUser('del-other@example.com'); + $device = $this->makeDevice('AA:BB:CC:DD:EE:F1', $owner); + $deviceId = $device->getId(); + + $client = $this->loginAs($other); + $client->request('DELETE', '/api/devices/' . $deviceId); + + $this->assertResponseStatusCodeSame(404); + + // Owner's device must still exist — no cross-tenant nuking. + $this->em()->clear(); + $this->assertNotNull($this->em()->find(\App\Entity\Device::class, $deviceId)); + } + + public function test_delete_unauthenticated_returns_unauthorized(): void + { + $owner = $this->createUser('del-anon@example.com'); + $device = $this->makeDevice('AA:BB:CC:DD:EE:F2', $owner); + + $this->client->request('DELETE', '/api/devices/' . $device->getId()); + + // Anon hits ROLE_USER firewall first — login redirect. + $this->assertResponseRedirects(); + } + + // After the seller deletes the device, the next poll from the physical + // hardware (with that MAC) sees an unknown MAC and the firmware shows the + // setup QR. Verifies the "delete leaves no server-side claim" guarantee. + public function test_after_delete_device_poll_returns_404(): void + { + $owner = $this->createUser('del-poll@example.com'); + $device = $this->makeDevice('AA:BB:CC:DD:EE:F3', $owner); + $mac = $device->getMac(); + + $client = $this->loginAs($owner); + $client->request('DELETE', '/api/devices/' . $device->getId()); + $this->assertResponseStatusCodeSame(204); + + // Anonymous device-poll endpoint — same MAC, must now 404. + $client->request('GET', '/api/device/' . $mac . '/image'); + $this->assertResponseStatusCodeSame(404); + } + public function test_preview_404_when_image_is_soft_deleted(): void { $user = $this->createUser('pdeleted@example.com');