From 08d0968af0b96b9031457fa70a688b46b75f78a9 Mon Sep 17 00:00:00 2001 From: Matt Edholm Date: Fri, 8 May 2026 18:51:31 -0400 Subject: [PATCH] feat(setup): post-link redirects to SPA so first-setup matches live UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twig configure page replaced with a redirect: SetupController's index, register, login, and the legacy /configure route all post-link redirect to /?setup= for unconfigured devices. The SPA's HomeView auto-opens its existing settings sheet for that id, with the same controls everyone uses for live edits — themed to the user's choice, pre-populated from the device record. Fixes Matt's report: - "every 6 hours" lost on save: the configure form posted rotation_interval_hours but the controller read rotation_interval_minutes, so the value silently defaulted to 1440 every time. Now the SPA's PATCH flow handles it correctly. - "old settings still there in live settings": SPA settings sheet pre-populates from the device's current state via onEdit. - "uniqueness window in setup but not live settings": removed from the (now-deleted) Twig form; both surfaces are consistent. - "color scheme didn't match account": SPA respects the user's theme natively (data-theme on ), so the first-setup screen looks like the rest of the app. Also adds a "Sign out of pictureFrame" link at the bottom of the per-frame settings sheet (the existing /settings tab still has the primary one). Easy escape hatch from a deeply-nested settings flow. Tests: - SetupControllerTest: S-03/04/05/06/08 updated for new redirect targets, S-CLAIM-03 updated. - HomeView.test.ts: useRoute now mockable per-test, two new cases pinning the ?setup= auto-open and its absence. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/src/test/views/HomeView.test.ts | 43 ++++++++++- frontend/src/views/HomeView.vue | 38 +++++++++- ...J-4S2HL.js => BaseBottomSheet-DKEsd9DM.js} | 2 +- ...r-qeCTtRmk.js => DevicePicker-DRpoyRVH.js} | 2 +- ...iew-WQheOzw3.css => HomeView-CwqIqvBo.css} | 2 +- public/build/assets/HomeView-D07qrHpY.js | 1 - public/build/assets/HomeView-eb15wpSB.js | 1 + ...ew-Czb8zdcL.js => LibraryView-DaEXQuDs.js} | 2 +- ...w-CX4Yi75h.js => SettingsView-Cm-jgVS7.js} | 2 +- ...iew-DaSwla70.js => UploadView-BeZPlVXi.js} | 2 +- .../{index-CmcHGdN5.js => index-DX-aWmo9.js} | 4 +- public/build/index.html | 2 +- src/Controller/SetupController.php | 60 +++++++-------- .../Controller/SetupControllerTest.php | 73 +++++++------------ 14 files changed, 139 insertions(+), 95 deletions(-) rename public/build/assets/{BaseBottomSheet-BJ-4S2HL.js => BaseBottomSheet-DKEsd9DM.js} (98%) rename public/build/assets/{DevicePicker-qeCTtRmk.js => DevicePicker-DRpoyRVH.js} (96%) rename public/build/assets/{HomeView-WQheOzw3.css => HomeView-CwqIqvBo.css} (71%) delete mode 100644 public/build/assets/HomeView-D07qrHpY.js create mode 100644 public/build/assets/HomeView-eb15wpSB.js rename public/build/assets/{LibraryView-Czb8zdcL.js => LibraryView-DaEXQuDs.js} (98%) rename public/build/assets/{SettingsView-CX4Yi75h.js => SettingsView-Cm-jgVS7.js} (92%) rename public/build/assets/{UploadView-DaSwla70.js => UploadView-BeZPlVXi.js} (98%) rename public/build/assets/{index-CmcHGdN5.js => index-DX-aWmo9.js} (99%) diff --git a/frontend/src/test/views/HomeView.test.ts b/frontend/src/test/views/HomeView.test.ts index 05d1ee6..259613b 100644 --- a/frontend/src/test/views/HomeView.test.ts +++ b/frontend/src/test/views/HomeView.test.ts @@ -49,9 +49,13 @@ vi.mock('@/components/OrientationPicker.vue', () => ({ }, })) -// Stub vue-router so HomeView can call useRouter() without a real router +// Stub vue-router so HomeView can call useRouter() without a real router. +// `mockRoute` is mutated per-test where needed (e.g. to surface ?setup=). +const mockRoute: { query: Record } = { query: {} } +const routerReplace = vi.fn() vi.mock('vue-router', () => ({ - useRouter: () => ({ push: routerPush }), + useRouter: () => ({ push: routerPush, replace: routerReplace }), + useRoute: () => mockRoute, })) // Stub URL.createObjectURL used by upload store @@ -1128,6 +1132,41 @@ describe('HomeView', () => { expect(document.querySelector('.home-view__remove-modal')).toBeNull() }) + // First-time setup: the SetupController redirects new claim/register + // flows to /?setup=. The SPA must auto-open the per-frame settings + // sheet for that device, with the standard pre-population, so the user + // never has to learn a separate Twig configure form. + it('auto-opens the per-frame settings sheet when ?setup= is in the URL', async () => { + const devicesStore = useDevicesStore() + devicesStore.devices = [makeDevice({ id: 7, name: '' })] + vi.spyOn(devicesStore, 'fetchDevices').mockResolvedValue() + + // Simulate landing on /?setup=7 (SetupController's post-link redirect). + mockRoute.query = { setup: '7' } + + const wrapper = mountView() + await flushPromises() + + const sheet = wrapper.findComponent({ name: 'BaseBottomSheet' }) + expect(sheet.props('modelValue')).toBe(true) + expect(routerReplace).toHaveBeenCalled() // query cleaned off the URL + + mockRoute.query = {} + }) + + it('does NOT open the sheet when no ?setup= param is present', async () => { + const devicesStore = useDevicesStore() + devicesStore.devices = [makeDevice({ id: 7 })] + vi.spyOn(devicesStore, 'fetchDevices').mockResolvedValue() + mockRoute.query = {} + + const wrapper = mountView() + await flushPromises() + + const sheet = wrapper.findComponent({ name: 'BaseBottomSheet' }) + expect(sheet.props('modelValue')).toBe(false) + }) + // Update the existing propagation-note text test to assert the new // "disconnect power" hint that lets users force an immediate refresh. it('propagation note mentions disconnecting power as an immediate-refresh gesture', async () => { diff --git a/frontend/src/views/HomeView.vue b/frontend/src/views/HomeView.vue index 6be5d16..2075412 100644 --- a/frontend/src/views/HomeView.vue +++ b/frontend/src/views/HomeView.vue @@ -231,6 +231,8 @@ class="home-view__remove" @click="removeConfirmOpen = true" >Remove this frame + + Sign out of pictureFrame @@ -279,7 +281,7 @@ + diff --git a/src/Controller/SetupController.php b/src/Controller/SetupController.php index 77b89a6..2a72274 100644 --- a/src/Controller/SetupController.php +++ b/src/Controller/SetupController.php @@ -39,10 +39,7 @@ class SetupController extends AbstractController $user = $this->getUser(); if (!$deviceService->isClaimedByAnotherUser($mac, $user)) { $device = $deviceService->linkToUser($mac, $user); - if (empty($device->getName())) { - return $this->redirectToRoute('setup_configure', ['mac' => $mac]); - } - return $this->redirectToRoute('spa'); + return $this->postLinkRedirect($device); } } @@ -94,7 +91,7 @@ class SetupController extends AbstractController $allowClaim = $request->request->getBoolean('claim_device'); try { - $deviceService->linkToUser($mac, $user, $allowClaim); + $device = $deviceService->linkToUser($mac, $user, $allowClaim); } catch (DeviceClaimRequiredException) { // New account just created and claim wasn't acknowledged. // Bounce back through the setup page; the checkbox will be @@ -108,7 +105,7 @@ class SetupController extends AbstractController return $this->redirectToRoute('setup_index', ['mac' => $mac]); } - return $this->redirectToRoute('setup_configure', ['mac' => $mac]); + return $this->postLinkRedirect($device); } $existing = $em->getRepository(Device::class)->findOneBy(['mac' => strtoupper($mac)]); @@ -138,7 +135,7 @@ class SetupController extends AbstractController $security->login($user, 'form_login', 'main'); $allowClaim = $request->request->getBoolean('claim_device'); try { - $deviceService->linkToUser($mac, $user, $allowClaim); + $device = $deviceService->linkToUser($mac, $user, $allowClaim); } catch (DeviceClaimRequiredException) { $request->getSession()->set( '_setup_claim_error', @@ -146,50 +143,43 @@ class SetupController extends AbstractController ); return $this->redirectToRoute('setup_index', ['mac' => $mac]); } - return $this->redirectToRoute('setup_configure', ['mac' => $mac]); + return $this->postLinkRedirect($device); } $request->getSession()->set('_setup_login_error', 'Incorrect email or password'); return $this->redirectToRoute('setup_index', ['mac' => $mac]); } + /** + * Legacy route — first-time configuration now happens in the SPA's + * settings sheet (auto-opens via the ?setup= query param) so users + * see the same controls as live editing, with their theme applied, + * pre-populated from the device's current state. Kept as a redirect so + * any in-flight bookmarks land in the right place. + */ #[Route('/configure', name: 'setup_configure', methods: ['GET', 'POST'])] #[IsGranted('ROLE_USER')] public function configure( string $mac, - Request $request, - EntityManagerInterface $em, DeviceService $deviceService, ): Response { /** @var User $user */ $user = $this->getUser(); $device = $deviceService->linkToUser($mac, $user); + return $this->postLinkRedirect($device); + } - if ($request->isMethod('POST')) { - $name = trim((string) $request->request->get('name', '')); - $orient = $request->request->get('orientation', Orientation::Landscape->value); - $interval = (int) $request->request->get('rotation_interval_minutes', 1440); - $window = (int) $request->request->get('uniqueness_window', 10); - - if (empty($name)) { - return $this->render('setup/configure.html.twig', [ - 'device' => $device, - 'error' => 'Please enter a name for your frame.', - ]); - } - - $device->setName($name); - $device->setOrientation(Orientation::from($orient)); - $device->setRotationIntervalMinutes(max(1, $interval)); - $device->setUniquenessWindow(max(1, $window)); - $em->flush(); - - return $this->redirectToRoute('spa'); + /** + * After linkToUser, hand off to the SPA. If the device hasn't been + * named yet (= first-time setup), append ?setup= so the SPA + * auto-opens its settings sheet for that device. Avoids duplicating + * the rich settings UI in a plain-Twig setup page. + */ + private function postLinkRedirect(Device $device): Response + { + if (empty($device->getName())) { + return $this->redirect('/?setup=' . $device->getId()); } - - return $this->render('setup/configure.html.twig', [ - 'device' => $device, - 'error' => null, - ]); + return $this->redirectToRoute('spa'); } } diff --git a/tests/Functional/Controller/SetupControllerTest.php b/tests/Functional/Controller/SetupControllerTest.php index bd381b3..759c76b 100644 --- a/tests/Functional/Controller/SetupControllerTest.php +++ b/tests/Functional/Controller/SetupControllerTest.php @@ -40,19 +40,22 @@ class SetupControllerTest extends AppWebTestCase $this->assertResponseRedirects('/'); } - // S-03: authenticated with no existing device (new link) → redirects to configure - public function test_setup_index_authenticated_new_device_redirects_to_configure(): void + // S-03: authenticated with no existing device → links and redirects to + // /?setup= so the SPA opens its settings sheet for first-time setup. + public function test_setup_index_authenticated_new_device_redirects_to_spa_setup(): void { $user = $this->createUser('setup03@example.com'); $this->loginAs($user); $this->client->request('GET', '/setup/' . self::MAC); - $this->assertResponseRedirects('/setup/' . self::MAC . '/configure'); + $this->assertResponseRedirects(); + $location = $this->client->getResponse()->headers->get('Location'); + $this->assertStringContainsString('/?setup=', $location, 'redirects to SPA with setup query'); } - // S-04: POST /setup/{mac}/register with valid data → creates user, links device, redirects to configure - public function test_setup_register_creates_user_and_redirects_to_configure(): void + // S-04: POST /register links + redirects to SPA with ?setup=. + public function test_setup_register_creates_user_and_redirects_to_spa_setup(): void { $this->client->request('POST', '/setup/' . self::MAC . '/register', [ 'registration_form' => [ @@ -64,11 +67,13 @@ class SetupControllerTest extends AppWebTestCase ], ]); - $this->assertResponseRedirects('/setup/' . self::MAC . '/configure'); + $this->assertResponseRedirects(); + $this->assertStringContainsString('/?setup=', $this->client->getResponse()->headers->get('Location')); } - // S-05: POST /setup/{mac}/configure with valid name → saves device, redirects to spa - public function test_setup_configure_saves_name_and_redirects_to_spa(): void + // S-05: legacy /configure POST is now a redirect to the SPA — the + // first-time settings UI lives in the live PWA, not Twig. + public function test_legacy_configure_post_redirects_to_spa_setup(): void { $user = $this->createUser('setup05@example.com'); @@ -78,25 +83,18 @@ class SetupControllerTest extends AppWebTestCase $this->em()->persist($device); $this->em()->flush(); - $deviceId = $device->getId(); - $this->loginAs($user); + // Old form POST — controller doesn't process the body, just redirects. $this->client->request('POST', '/setup/' . self::MAC . '/configure', [ - 'name' => 'Kitchen Frame', - 'orientation' => 'landscape', - 'rotation_interval_minutes' => '1440', - 'uniqueness_window' => '10', + 'name' => 'Kitchen Frame', ]); - $this->assertResponseRedirects('/'); - - $this->em()->clear(); - $saved = $this->em()->find(Device::class, $deviceId); - $this->assertSame('Kitchen Frame', $saved->getName()); + $this->assertResponseRedirects(); + $this->assertStringContainsString('/?setup=', $this->client->getResponse()->headers->get('Location')); } - // S-06: POST /setup/{mac}/login with valid credentials → redirects to configure - public function test_login_valid_credentials_redirects_to_configure(): void + // S-06: /login → links + redirects to SPA setup. + public function test_login_valid_credentials_redirects_to_spa_setup(): void { $this->createUser('setuplogin@example.com', 'testpass'); @@ -105,7 +103,8 @@ class SetupControllerTest extends AppWebTestCase '_password' => 'testpass', ]); - $this->assertResponseRedirects('/setup/' . self::MAC . '/configure'); + $this->assertResponseRedirects(); + $this->assertStringContainsString('/?setup=', $this->client->getResponse()->headers->get('Location')); } // S-07: POST /setup/{mac}/login with wrong password → redirects to index @@ -121,8 +120,9 @@ class SetupControllerTest extends AppWebTestCase $this->assertResponseRedirects('/setup/' . self::MAC); } - // S-08: authenticated GET /setup/{mac}/configure → 200 - public function test_configure_get_renders_form(): void + // S-08: GET /setup/{mac}/configure also redirects (legacy, kept for + // any in-flight bookmarks). + public function test_legacy_configure_get_redirects_to_spa(): void { $user = $this->createUser('setupconfigget@example.com'); @@ -135,28 +135,10 @@ class SetupControllerTest extends AppWebTestCase $this->loginAs($user); $this->client->request('GET', '/setup/' . self::MAC . '/configure'); - $this->assertResponseIsSuccessful(); + $this->assertResponseRedirects(); + $this->assertStringContainsString('/?setup=', $this->client->getResponse()->headers->get('Location')); } - // S-09: POST /setup/{mac}/configure with empty name → 200 (re-renders form with error) - public function test_configure_post_empty_name_renders_error(): void - { - $user = $this->createUser('setupconfigerr@example.com'); - - $device = new Device(); - $device->setMac(self::MAC); - $device->setUser($user); - $this->em()->persist($device); - $this->em()->flush(); - - $this->loginAs($user); - $this->client->request('POST', '/setup/' . self::MAC . '/configure', [ - 'name' => '', - 'orientation' => 'landscape', - ]); - - $this->assertResponseIsSuccessful(); - } // S-10: POST /setup/{mac}/register with invalid form data → re-renders registration page public function test_setup_register_invalid_form_renders_page(): void @@ -260,7 +242,8 @@ class SetupControllerTest extends AppWebTestCase ], 'claim_device' => '1', ]); - $this->assertResponseRedirects('/setup/' . self::MAC . '/configure'); + $this->assertResponseRedirects(); + $this->assertStringContainsString('/?setup=', $this->client->getResponse()->headers->get('Location')); $this->em()->clear(); $reloaded = $this->em()->find(Device::class, $deviceId);