From 0e701e839b96961fd3333f13abb172816fce15e2 Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Sun, 16 Aug 2026 00:27:15 -0500 Subject: [PATCH] Move service selection to Advanced --- src/css/style.css | 13 +- src/js/config.js | 16 ++ src/js/push/courierPushService.js | 77 +++++-- src/js/views/settings/advanced.view.js | 181 ++++++++++++++- src/js/views/settings/notifications.view.js | 217 ++++------------- .../views/settings/advanced.view.test.js | 218 +++++++++++++++++- .../views/settingsNotifications.view.test.js | 148 +++--------- tests/unit/specs/courierPushService.test.js | 43 +++- 8 files changed, 607 insertions(+), 306 deletions(-) diff --git a/src/css/style.css b/src/css/style.css index 53e830a1..d0915c59 100644 --- a/src/css/style.css +++ b/src/css/style.css @@ -7837,6 +7837,10 @@ context-menu-item-group > context-menu-item > button { margin-bottom: 16px; } +.settings-section p a { + color: var(--text-link-color); +} + .settings-color-picker { display: flex; flex-direction: column; @@ -10781,8 +10785,9 @@ rendered-markdown ul:has(> li > input[type="checkbox"]) { border-bottom: var(--hair) solid var(--generic-border-color); } -.setting-item-disabled .setting-item-info { - opacity: 0.6; +.setting-item-disabled .setting-item-name, +.setting-item-disabled .setting-item-desc { + color: var(--text-color-muted); } .setting-item:last-child { @@ -10812,6 +10817,10 @@ h2.setting-item-name { margin-top: 4px; } +.setting-item-desc a { + color: var(--text-link-color); +} + .plugin-content .setting-item-name, .plugin-content .setting-item-desc { padding: 0; diff --git a/src/js/config.js b/src/js/config.js index b9912c25..b6bb7e32 100644 --- a/src/js/config.js +++ b/src/js/config.js @@ -61,3 +61,19 @@ export const DEFAULT_APP_VIEW_CONFIGS = [ AppViewConfig.BLUESKY, AppViewConfig.BLACKSKY, ]; + +export const NONE_NOTIFICATION_SERVICE_ID = "none"; +export const CUSTOM_NOTIFICATION_SERVICE_ID = "custom"; + +export const NOTIFICATION_SERVICE_PRESETS = [ + { + id: NONE_NOTIFICATION_SERVICE_ID, + displayName: "None", + serviceDid: null, + }, + { + id: "courier-7778777", + displayName: "7778777.online/courier", + serviceDid: "did:web:courier.7778777.online", + }, +]; diff --git a/src/js/push/courierPushService.js b/src/js/push/courierPushService.js index 0489da72..29088e58 100644 --- a/src/js/push/courierPushService.js +++ b/src/js/push/courierPushService.js @@ -1,4 +1,5 @@ import { resolveDid, getServiceEndpointFromDidDoc } from "/js/atproto.js"; +import { Signal } from "/js/signals.js"; const STORAGE_KEY = "courier-push-enabled"; const PREVIEWS_KEY = "courier-push-chat-previews"; @@ -53,6 +54,15 @@ export class CourierPushService { constructor(api) { this.api = api; this._configPromise = null; + this.$serviceDid = new Signal.State( + localStorage.getItem(SERVICE_KEY) || null, + ); + this.$enabled = new Signal.State( + localStorage.getItem(STORAGE_KEY) === "true", + ); + this.$chatPreviews = new Signal.State( + localStorage.getItem(PREVIEWS_KEY) === "true", + ); } get isSupported() { @@ -64,18 +74,14 @@ export class CourierPushService { } get isEnabled() { - return ( - this.isSupported && - this.hasService && - localStorage.getItem(STORAGE_KEY) === "true" - ); + return this.isSupported && this.hasService && this.$enabled.get(); } // The service this device is pointed at, or null if the user has not named // one. Impro suggests none: a service holds a read-only grant over the // account and polls on the user's behalf, so the choice is always theirs. get serviceDid() { - return localStorage.getItem(SERVICE_KEY) || null; + return this.$serviceDid.get(); } // Nothing can be enabled, resolved or registered until the user has named a @@ -89,7 +95,34 @@ export class CourierPushService { // have changed it; only the echo is truthful, and it self-corrects every // time the flow runs. get chatPreviewsEnabled() { - return localStorage.getItem(PREVIEWS_KEY) === "true"; + return this.$chatPreviews.get(); + } + + _setServiceDid(did) { + if (did === null) { + localStorage.removeItem(SERVICE_KEY); + } else { + localStorage.setItem(SERVICE_KEY, did); + } + this.$serviceDid.set(did); + } + + _setEnabled(enabled) { + if (enabled) { + localStorage.setItem(STORAGE_KEY, "true"); + } else { + localStorage.removeItem(STORAGE_KEY); + } + this.$enabled.set(enabled); + } + + _setChatPreviews(chatPreviews) { + if (chatPreviews) { + localStorage.setItem(PREVIEWS_KEY, "true"); + } else { + localStorage.removeItem(PREVIEWS_KEY); + } + this.$chatPreviews.set(chatPreviews); } // Resolves a service DID far enough to show the user what they are about to @@ -111,7 +144,16 @@ export class CourierPushService { await this.disable(); } this._forgetConfig(); - localStorage.setItem(SERVICE_KEY, did); + this._setServiceDid(did); + } + + async clearService() { + if (!this.hasService) return; + if (this.isEnabled) { + await this.disable(); + } + this._forgetConfig(); + this._setServiceDid(null); } async fetchServiceConfig() { @@ -220,7 +262,7 @@ export class CourierPushService { async completeEnableFlow({ chatPreviews = false } = {}) { const config = await this.fetchServiceConfig(); await this._subscribeAndRegister(config); - localStorage.setItem(PREVIEWS_KEY, chatPreviews ? "true" : "false"); + this._setChatPreviews(chatPreviews); this.startHeartbeat(); } @@ -244,22 +286,17 @@ export class CourierPushService { platform: PLATFORM, appId: APP_ID, }); - localStorage.setItem(STORAGE_KEY, "true"); + this._setEnabled(true); } // Re-assert registration on every app launch: registerPush is an // idempotent upsert and there is no API to query registration state, so // this is the self-healing path for a rotated or lost subscription. async reassertIfEnabled() { - if ( - localStorage.getItem(STORAGE_KEY) !== "true" || - !this.isSupported || - !this.hasService - ) - return; + if (!this.$enabled.get() || !this.isSupported || !this.hasService) return; if (Notification.permission !== "granted") { // Permission was revoked out-of-band (browser site settings). - localStorage.removeItem(STORAGE_KEY); + this._setEnabled(false); return; } try { @@ -317,7 +354,7 @@ export class CourierPushService { if (now - (this._lastHeartbeat ?? 0) < HEARTBEAT_INTERVAL_MS / 2) return; this._lastHeartbeat = now; - if (localStorage.getItem(STORAGE_KEY) !== "true" || !this.isSupported) { + if (!this.$enabled.get() || !this.isSupported) { this.stopHeartbeat(); return; } @@ -358,8 +395,8 @@ export class CourierPushService { // for a logged-out account from reaching this device). async disable() { this.stopHeartbeat(); - localStorage.removeItem(STORAGE_KEY); - localStorage.removeItem(PREVIEWS_KEY); + this._setEnabled(false); + this._setChatPreviews(false); if (!("serviceWorker" in navigator)) return; const registration = await navigator.serviceWorker.getRegistration(SW_PATH); const subscription = await registration?.pushManager.getSubscription(); diff --git a/src/js/views/settings/advanced.view.js b/src/js/views/settings/advanced.view.js index 456544f8..68a8dfea 100644 --- a/src/js/views/settings/advanced.view.js +++ b/src/js/views/settings/advanced.view.js @@ -2,7 +2,12 @@ import { html, render } from "/js/lib/lit-html.js"; import { pageEffect, bindPageTitle } from "/js/router.js"; import { headerTemplate } from "/js/templates/header.template.js"; import { auth } from "/js/auth.js"; -import { AppViewConfig, DEFAULT_APP_VIEW_CONFIGS } from "/js/config.js"; +import { + AppViewConfig, + DEFAULT_APP_VIEW_CONFIGS, + NOTIFICATION_SERVICE_PRESETS, + CUSTOM_NOTIFICATION_SERVICE_ID, +} from "/js/config.js"; import { getAppViewConfig, setAppViewConfig, @@ -18,7 +23,7 @@ export default async function settingsAdvancedView({ root, router, layout, - context: { pluginService }, + context: { pluginService, courierPushService }, }) { await auth.requireAuth(); @@ -36,6 +41,23 @@ export default async function settingsAdvancedView({ isStoredCustom ? storedConfig.chatServiceDid : "", ); state.$pluginInstallLoading = new Signal.State(false); + const storedServiceDid = courierPushService?.serviceDid ?? null; + const storedServicePreset = NOTIFICATION_SERVICE_PRESETS.find( + (preset) => preset.serviceDid === storedServiceDid, + ); + const storedServiceSelection = + storedServicePreset?.id ?? CUSTOM_NOTIFICATION_SERVICE_ID; + + state.$notificationServiceLoading = new Signal.State(false); + state.$notificationServiceError = new Signal.State(null); + state.$notificationServiceSelection = new Signal.State( + storedServiceSelection, + ); + state.$customNotificationServiceDid = new Signal.State( + storedServiceSelection === CUSTOM_NOTIFICATION_SERVICE_ID + ? storedServiceDid + : "", + ); function resolveSelectedAppViewConfig() { if (state.$appViewSelection.get() === CUSTOM_APP_VIEW_CONFIG_ID) { @@ -89,6 +111,71 @@ export default async function settingsAdvancedView({ state.$customChatServiceDid.set(e.target.value); } + function resolveSelectedServiceDid() { + const selection = state.$notificationServiceSelection.get(); + if (selection === CUSTOM_NOTIFICATION_SERVICE_ID) { + return state.$customNotificationServiceDid.get().trim(); + } + return ( + NOTIFICATION_SERVICE_PRESETS.find((preset) => preset.id === selection) + ?.serviceDid ?? null + ); + } + + function isNotificationServiceDirty() { + return ( + resolveSelectedServiceDid() !== (courierPushService?.serviceDid ?? null) + ); + } + + function handleNotificationServiceChange(e) { + state.$notificationServiceSelection.set(e.target.value); + state.$notificationServiceError.set(null); + } + + function handleCustomNotificationServiceDidInput(e) { + state.$customNotificationServiceDid.set(e.target.value); + } + + async function handleNotificationServiceSubmit(e) { + e.preventDefault(); + if (!courierPushService) return; + const did = resolveSelectedServiceDid(); + state.$notificationServiceError.set(null); + + if (did === null) { + state.$notificationServiceLoading.set(true); + try { + await courierPushService.clearService(); + showToast("Selected notification service: None", { style: "success" }); + } finally { + state.$notificationServiceLoading.set(false); + } + return; + } + + if (!did.startsWith("did:")) { + state.$notificationServiceError.set("Please enter a valid DID."); + return; + } + state.$notificationServiceLoading.set(true); + try { + // Resolve before switching: an unreachable or non-conforming service + // should fail here, not after the current one has been torn down. + const { name } = await courierPushService.previewService(did); + const wasEnabled = courierPushService.isEnabled; + await courierPushService.selectService(did); + showToast(`Selected notification service: ${name}`, { style: "success" }); + } catch (error) { + console.error(error); + state.$notificationServiceError.set( + "Couldn't reach that service, or it isn't a notification service.", + ); + } finally { + state.$notificationServiceLoading.set(false); + } + } + async function handleInstallPlugin(e) { e.preventDefault(); const input = e.target.elements.pluginUrl; @@ -117,6 +204,10 @@ export default async function settingsAdvancedView({ pageEffect(root, () => { const isCustom = state.$appViewSelection.get() === CUSTOM_APP_VIEW_CONFIG_ID; + const notificationServiceSelection = + state.$notificationServiceSelection.get(); + const isCustomNotificationService = + notificationServiceSelection === CUSTOM_NOTIFICATION_SERVICE_ID; render( html`
${headerTemplate({ @@ -225,6 +316,92 @@ export default async function settingsAdvancedView({
+
handleNotificationServiceSubmit(e)} + > +
+

Push notification service

+

+ Choose an external service to deliver push notifications. + Notification Settings +

+
+
+ +
+
+ ${isCustomNotificationService + ? html` +
+ + + handleCustomNotificationServiceDidInput(e)} + /> +
+ ` + : ""} +
+ +
+
+ ${state.$notificationServiceError.get() + ? html`
+ ${state.$notificationServiceError.get()} +
` + : ""} +
+
+
handleInstallPlugin(e)} diff --git a/src/js/views/settings/notifications.view.js b/src/js/views/settings/notifications.view.js index f352b0e3..b9600330 100644 --- a/src/js/views/settings/notifications.view.js +++ b/src/js/views/settings/notifications.view.js @@ -36,26 +36,20 @@ export default async function settingsNotificationsView({ const $enabled = new Signal.State( systemNotificationService?.isEnabled ?? false, ); - const $pushEnabled = new Signal.State(courierPushService?.isEnabled ?? false); - const $chatPreviews = new Signal.State( - courierPushService?.chatPreviewsEnabled ?? false, - ); const $pushBusy = new Signal.State(false); // The service is user-selectable per the spec, so its name is data, not a // constant — every mention of it in this view comes from the service's own // config document. Falls back to the DID until that resolves. - const $serviceDid = new Signal.State(courierPushService?.serviceDid ?? null); const $serviceName = new Signal.State(courierPushService?.serviceDid ?? null); - const $pickerOpen = new Signal.State(false); - const $pickerValue = new Signal.State(""); - const $pickerBusy = new Signal.State(false); - const $pickerError = new Signal.State(""); - async function loadServiceName() { - if (!courierPushService?.hasService) return; - const did = courierPushService.serviceDid; - $serviceDid.set(did); + async function loadServiceName(did) { + if (did === null) { + $serviceName.set(null); + return; + } + // Show the DID until the lookup resolves. + $serviceName.set(did); try { const { name } = await courierPushService.previewService(did); // Guard against a slow lookup landing after the user switched again. @@ -66,43 +60,13 @@ export default async function settingsNotificationsView({ if (courierPushService.serviceDid === did) $serviceName.set(did); } } - loadServiceName(); - async function handleServiceSelect() { - const did = $pickerValue.get().trim(); - if (!did || !courierPushService) return; - if (!did.startsWith("did:")) { - $pickerError.set("That doesn't look like a DID."); - return; - } - $pickerBusy.set(true); - $pickerError.set(""); - try { - // Resolve before switching: an unreachable or non-conforming service - // should fail here, not after the current one has been torn down. - const { name } = await courierPushService.previewService(did); - const wasEnabled = courierPushService.isEnabled; - await courierPushService.selectService(did); - $pushEnabled.set(courierPushService.isEnabled); - $chatPreviews.set(courierPushService.chatPreviewsEnabled); - $serviceDid.set(did); - $serviceName.set(name); - $pickerOpen.set(false); - $pickerValue.set(""); - showToast( - wasEnabled - ? `Switched to ${name}. Turn push notifications back on to finish.` - : `Switched to ${name}.`, - ); - } catch (error) { - console.error(error); - $pickerError.set( - "Couldn't reach that service, or it isn't a notification service.", - ); - } finally { - $pickerBusy.set(false); - } - } + // Tracks the service's own DID signal, so choosing one on Advanced reaches + // this page without it having to re-read anything on navigation. Kept out of + // the render effect so the lookup isn't refired by unrelated re-renders. + pageEffect(root, () => { + loadServiceName(courierPushService?.$serviceDid.get() ?? null); + }); async function handleToggle(checked) { if (!systemNotificationService) return; @@ -111,11 +75,6 @@ export default async function settingsNotificationsView({ $enabled.set(false); return; } - const confirmed = await confirmModal( - "Impro will ask your browser for permission to show notifications. You can turn this off again at any time.", - { title: "Enable notifications?", confirmButtonText: "Continue" }, - ); - if (!confirmed) return; const result = await systemNotificationService.requestPermission(); if (result === "granted") { $enabled.set(true); @@ -139,7 +98,6 @@ export default async function settingsNotificationsView({ } finally { $pushBusy.set(false); } - $pushEnabled.set(false); return; } const confirmed = await confirmModal( @@ -149,7 +107,7 @@ export default async function settingsNotificationsView({ if (!confirmed) return; try { await courierPushService.startEnableFlow({ - chatPreviews: $chatPreviews.get(), + chatPreviews: courierPushService.chatPreviewsEnabled, }); } catch (error) { console.error(error); @@ -201,8 +159,6 @@ export default async function settingsNotificationsView({ await courierPushService.completeEnableFlow({ chatPreviews: callback.chatPreviews, }); - $pushEnabled.set(true); - $chatPreviews.set(callback.chatPreviews); showToast("Push notifications enabled."); } catch (error) { console.error(error); @@ -234,22 +190,19 @@ export default async function settingsNotificationsView({ "Notifications are blocked for this site. Re-enable them in your browser's site settings to turn this on."; } - const pushEnabled = $pushEnabled.get(); + const pushEnabled = courierPushService?.isEnabled ?? false; const pushBusy = $pushBusy.get(); - const chatPreviews = $chatPreviews.get(); + const chatPreviews = courierPushService?.chatPreviewsEnabled ?? false; const pushSupported = courierPushService?.isSupported ?? false; const serviceName = $serviceName.get(); - const serviceDid = $serviceDid.get(); - const pickerOpen = $pickerOpen.get(); - const pickerBusy = $pickerBusy.get(); - const pickerError = $pickerError.get(); + const serviceDid = courierPushService?.$serviceDid.get() ?? null; const hasService = serviceDid !== null; - let pushDescription = "Your browser doesn't support push notifications."; - if (pushSupported) { - pushDescription = hasService - ? `Get notified even when Impro is closed, via ${serviceName}.` - : "Get notified even when Impro is closed. Choose a notification service below to turn this on."; - } + + // One source of truth per row, so the dimmed style and the toggle's own + // disabled state can't drift apart. + const systemRowDisabled = !isSupported || isDenied; + const pushRowDisabled = !pushSupported || !hasService || pushBusy; + const previewsRowDisabled = pushBusy; render( html`
@@ -260,12 +213,12 @@ export default async function settingsNotificationsView({
-

Enable desktop notifications

+

Desktop notifications

${description}

@@ -273,32 +226,49 @@ export default async function settingsNotificationsView({ data-testid="system-notifications-toggle" label="Enable notifications" ?checked=${enabled} - ?disabled=${!isSupported || isDenied} + ?disabled=${systemRowDisabled} @change=${(event) => handleToggle(event.detail.checked)} >
-

Push notifications

-

${pushDescription}

+

Push notifications (beta)

+

+ ${!pushSupported + ? "Your browser doesn't support push notifications." + : hasService + ? "Get notified even when Impro is closed." + : html`Get notified even when Impro is closed. Choose a + notification service under + Advanced + to enable this feature.`} +

handlePushToggle(event.detail.checked)} >
${pushEnabled ? html`
@@ -314,102 +284,13 @@ export default async function settingsNotificationsView({ data-testid="chat-previews-toggle" label="Show message previews" ?checked=${chatPreviews} - ?disabled=${pushBusy} + ?disabled=${previewsRowDisabled} @change=${(event) => handlePreviewsToggle(event.detail.checked)} >
` : null} - ${pushSupported - ? html`
-
-

Notification service

- ${hasService - ? html`

- Push notifications are delivered by - ${serviceName}, which holds a read-only - grant to watch this account's notifications on your - behalf. You can point Impro at a different service, or - run your own.
${serviceDid} -

` - : html`

- Impro doesn't pick a notification service for you. Enter - the DID of one you trust — or run your own — and it will - hold a read-only grant to watch this account's - notifications on your behalf. -

`} - ${pickerOpen - ? html`
- - $pickerValue.set(event.target.value)} - @keydown=${(event) => { - if (event.key === "Enter") handleServiceSelect(); - }} - /> -

- A notification service can read this account's - notifications, and its message content if you turn - previews on. Only use one you trust. -

- ${pickerError - ? html`

- ${pickerError} -

` - : null} - - -
` - : html``} -
-
` - : null}
`, root, diff --git a/tests/e2e/specs/views/settings/advanced.view.test.js b/tests/e2e/specs/views/settings/advanced.view.test.js index 6020c2b0..1d2513c9 100644 --- a/tests/e2e/specs/views/settings/advanced.view.test.js +++ b/tests/e2e/specs/views/settings/advanced.view.test.js @@ -1,6 +1,8 @@ +import assert from "node:assert/strict"; import { test, expect } from "../../../base.js"; -import { login } from "../../../helpers.js"; +import { login, selectNotificationService } from "../../../helpers.js"; import { MockServer } from "../../../mockServer.js"; +import { notificationService } from "../../../testData.js"; test.describe("Settings Advanced view", () => { test("should display header and App View section", async ({ page }) => { @@ -284,6 +286,220 @@ test.describe("Settings Advanced view", () => { }); }); + test.describe("Notification service section", () => { + test("defaults to None and sits between App View and Install plugin", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await expect(select).toHaveValue("none"); + await expect(select.locator("option")).toHaveText([ + "None", + "7778777.online/courier", + "Custom", + ]); + // None is inert, so neither the DID input nor the warning shows. + await expect( + page.locator('[data-testid="notification-service-input"]'), + ).toHaveCount(0); + await expect( + page.locator("#notification-service-form .warning-area"), + ).toHaveCount(0); + + const formIds = await page.evaluate(() => + [...document.querySelectorAll("#settings-advanced-view main form")].map( + (form) => form.id, + ), + ); + assert.deepEqual(formIds, [ + "settings-advanced-form", + "notification-service-form", + "install-unregistered-plugin-form", + ]); + }); + + test("save is disabled until the selection changes", async ({ page }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + const save = page.locator('[data-testid="notification-service-save"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await expect(save).toBeDisabled(); + + await select.selectOption("courier-7778777"); + await expect(save).toBeEnabled(); + + await select.selectOption("none"); + await expect(save).toBeDisabled(); + }); + + test("selecting a listed service needs no custom DID input", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await select.selectOption("courier-7778777"); + + // The preset already names a DID, so nothing is asked of the user. + await expect( + page.locator('[data-testid="notification-service-input"]'), + ).toHaveCount(0); + await expect( + page.locator('[data-testid="notification-service-save"]'), + ).toBeEnabled(); + }); + + test("Custom reveals the DID input and stores what is entered", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await select.selectOption("custom"); + + const input = page.locator('[data-testid="notification-service-input"]'); + await expect(input).toBeVisible(); + await input.fill(notificationService.did); + await page.locator('[data-testid="notification-service-save"]').click(); + + await expect(page.locator('[data-testid="toast"]')).toContainText( + notificationService.name, + ); + expect( + await page.evaluate(() => + localStorage.getItem("courier-push-service-did"), + ), + ).toBe(notificationService.did); + }); + + test("a stored service that matches no preset selects Custom and prefills it", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await selectNotificationService(page); + await page.goto("/settings/advanced"); + + await expect( + page.locator('select[name="notificationService"]'), + ).toHaveValue("custom", { timeout: 10000 }); + await expect( + page.locator('[data-testid="notification-service-input"]'), + ).toHaveValue(notificationService.did); + }); + + test("a stored preset DID selects that preset, not Custom", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await selectNotificationService(page, { + did: "did:web:courier.7778777.online", + }); + await page.goto("/settings/advanced"); + + await expect( + page.locator('select[name="notificationService"]'), + ).toHaveValue("courier-7778777", { timeout: 10000 }); + await expect( + page.locator('[data-testid="notification-service-input"]'), + ).toHaveCount(0); + }); + + test("selecting None clears the stored service", async ({ page }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await selectNotificationService(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toHaveValue("custom", { timeout: 10000 }); + + await select.selectOption("none"); + await page.locator('[data-testid="notification-service-save"]').click(); + + await expect + .poll(() => + page.evaluate(() => localStorage.getItem("courier-push-service-did")), + ) + .toBeNull(); + }); + + test("rejects input that isn't a DID without hitting the network", async ({ + page, + }) => { + const mockServer = new MockServer(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await select.selectOption("custom"); + await page + .locator('[data-testid="notification-service-input"]') + .fill("notifs.example.com"); + await page.locator('[data-testid="notification-service-save"]').click(); + + await expect( + page.locator('[data-testid="notification-service-error"]'), + ).toBeVisible(); + expect( + await page.evaluate(() => + localStorage.getItem("courier-push-service-did"), + ), + ).toBeNull(); + }); + + test("a service that won't resolve is rejected and not stored", async ({ + page, + }) => { + const mockServer = new MockServer(); + mockServer.failNotificationServiceLookup(); + await mockServer.setup(page); + await login(page); + await page.goto("/settings/advanced"); + + const select = page.locator('select[name="notificationService"]'); + await expect(select).toBeVisible({ timeout: 10000 }); + await select.selectOption("custom"); + await page + .locator('[data-testid="notification-service-input"]') + .fill(notificationService.did); + await page.locator('[data-testid="notification-service-save"]').click(); + + await expect( + page.locator('[data-testid="notification-service-error"]'), + ).toBeVisible({ timeout: 10000 }); + // The previous state must survive a failed switch, so nothing is stored. + expect( + await page.evaluate(() => + localStorage.getItem("courier-push-service-did"), + ), + ).toBeNull(); + }); + }); + test.describe("Logged-out behavior", () => { test("should redirect to /login when not authenticated", async ({ page, diff --git a/tests/e2e/specs/views/settingsNotifications.view.test.js b/tests/e2e/specs/views/settingsNotifications.view.test.js index 033c9b17..9ca40bc0 100644 --- a/tests/e2e/specs/views/settingsNotifications.view.test.js +++ b/tests/e2e/specs/views/settingsNotifications.view.test.js @@ -51,10 +51,6 @@ test.describe("Settings > Notifications view", () => { await toggle.click(); - const confirmModal = page.locator('[data-testid="confirm-modal"]'); - await expect(confirmModal).toBeVisible(); - await confirmModal.locator('[data-testid="modal-confirm-button"]').click(); - await expect(toggle).toHaveAttribute("checked", "", { timeout: 10000 }); await expect .poll(() => @@ -65,32 +61,6 @@ test.describe("Settings > Notifications view", () => { .toBe("true"); }); - test("declining the confirm dialog leaves notifications disabled", async ({ - page, - }) => { - const mockServer = new MockServer(); - await mockServer.setup(page); - await stubNotificationPermission(page, { initial: "default" }); - await login(page); - await page.goto("/settings/notifications"); - - const toggle = page.locator('[data-testid="system-notifications-toggle"]'); - await expect(toggle).toBeVisible({ timeout: 10000 }); - - await toggle.click(); - - const confirmModal = page.locator('[data-testid="confirm-modal"]'); - await expect(confirmModal).toBeVisible(); - await confirmModal.locator('[data-testid="modal-cancel-button"]').click(); - - await expect(toggle).not.toHaveAttribute("checked", ""); - expect( - await page.evaluate(() => - localStorage.getItem("system-notifications-enabled"), - ), - ).toBeNull(); - }); - test("shows an error toast when the browser denies permission", async ({ page, }) => { @@ -107,11 +77,6 @@ test.describe("Settings > Notifications view", () => { await expect(toggle).toBeVisible({ timeout: 10000 }); await toggle.click(); - await page - .locator( - '[data-testid="confirm-modal"] [data-testid="modal-confirm-button"]', - ) - .click(); await expect(page.locator('[data-testid="toast"]')).toBeVisible({ timeout: 10000, @@ -180,8 +145,8 @@ test.describe("Settings > Notifications view", () => { .toBeNull(); }); - test.describe("notification service selection", () => { - test("with no service chosen, push cannot be turned on", async ({ + test.describe("push notifications", () => { + test("with no service named, push cannot be turned on", async ({ page, }) => { const mockServer = new MockServer(); @@ -190,18 +155,18 @@ test.describe("Settings > Notifications view", () => { await login(page); await page.goto("/settings/notifications"); - await expect( - page.locator('[data-testid="notification-service-unset"]'), - ).toBeVisible({ timeout: 10000 }); + // The service is named under Advanced, so the row's subtitle links there. + const unset = page.locator( + '[data-testid="settings-section-push-notifications"] [data-testid="notification-service-unset"]', + ); + await expect(unset).toBeVisible({ timeout: 10000 }); + await expect(unset).toHaveAttribute("href", "/settings/advanced"); await expect( page.locator('[data-testid="push-notifications-toggle"]'), ).toHaveAttribute("disabled", ""); - await expect( - page.locator('[data-testid="notification-service-change"]'), - ).toHaveAttribute("data-teststate", "unset"); }); - test("entering a service DID resolves it and enables the push toggle", async ({ + test("picking a service on Advanced updates this page on the way back", async ({ page, }) => { const mockServer = new MockServer(); @@ -210,55 +175,37 @@ test.describe("Settings > Notifications view", () => { await login(page); await page.goto("/settings/notifications"); + const pushRow = page.locator( + '[data-testid="settings-section-push-notifications"]', + ); + await expect( + pushRow.locator('[data-testid="notification-service-unset"]'), + ).toBeVisible({ timeout: 10000 }); + await page - .locator('[data-testid="notification-service-change"]') + .locator('[data-testid="notification-service-unset"]') .click({ timeout: 10000 }); + await page + .locator('select[name="notificationService"]') + .selectOption("custom"); await page .locator('[data-testid="notification-service-input"]') .fill(notificationService.did); await page.locator('[data-testid="notification-service-save"]').click(); + await expect(page.locator('[data-testid="toast"]')).toBeVisible(); - await expect(page.locator('[data-testid="toast"]')).toContainText( - notificationService.name, - ); + // The page stays cached in the DOM, so it only updates because it reads + // the service's signals rather than a snapshot taken when it was built. + await page.goBack(); await expect( - page.locator('[data-testid="notification-service-unset"]'), - ).toHaveCount(0); + pushRow.locator('[data-testid="notification-service-unset"]'), + ).toHaveCount(0, { timeout: 10000 }); await expect( page.locator('[data-testid="push-notifications-toggle"]'), ).not.toHaveAttribute("disabled", ""); - expect( - await page.evaluate(() => - localStorage.getItem("courier-push-service-did"), - ), - ).toBe(notificationService.did); }); - test("a previously chosen service is shown by name on load", async ({ - page, - }) => { - const mockServer = new MockServer(); - await mockServer.setup(page); - await stubNotificationPermission(page, { initial: "default" }); - await login(page); - await selectNotificationService(page); - await page.goto("/settings/notifications"); - - const section = page.locator( - '[data-testid="settings-section-notification-service"]', - ); - await expect(section).toContainText(notificationService.name, { - timeout: 10000, - }); - await expect(section).toContainText(notificationService.did); - await expect( - page.locator('[data-testid="notification-service-change"]'), - ).toHaveAttribute("data-teststate", "set"); - }); - - test("rejects input that isn't a DID without hitting the network", async ({ - page, - }) => { + test("the subtitle link navigates to Advanced", async ({ page }) => { const mockServer = new MockServer(); await mockServer.setup(page); await stubNotificationPermission(page, { initial: "default" }); @@ -266,50 +213,29 @@ test.describe("Settings > Notifications view", () => { await page.goto("/settings/notifications"); await page - .locator('[data-testid="notification-service-change"]') + .locator('[data-testid="notification-service-unset"]') .click({ timeout: 10000 }); - await page - .locator('[data-testid="notification-service-input"]') - .fill("notifs.example.com"); - await page.locator('[data-testid="notification-service-save"]').click(); + await expect(page).toHaveURL(/\/settings\/advanced$/); await expect( - page.locator('[data-testid="notification-service-error"]'), + page.locator('select[name="notificationService"]'), ).toBeVisible(); - expect( - await page.evaluate(() => - localStorage.getItem("courier-push-service-did"), - ), - ).toBeNull(); }); - test("a service that won't resolve is rejected and not stored", async ({ - page, - }) => { + test("with a service named, the toggle is usable", async ({ page }) => { const mockServer = new MockServer(); - mockServer.failNotificationServiceLookup(); await mockServer.setup(page); await stubNotificationPermission(page, { initial: "default" }); await login(page); + await selectNotificationService(page); await page.goto("/settings/notifications"); - await page - .locator('[data-testid="notification-service-change"]') - .click({ timeout: 10000 }); - await page - .locator('[data-testid="notification-service-input"]') - .fill(notificationService.did); - await page.locator('[data-testid="notification-service-save"]').click(); - await expect( - page.locator('[data-testid="notification-service-error"]'), - ).toBeVisible({ timeout: 10000 }); - // The old state must survive a failed switch, so nothing is stored. - expect( - await page.evaluate(() => - localStorage.getItem("courier-push-service-did"), - ), - ).toBeNull(); + page.locator('[data-testid="push-notifications-toggle"]'), + ).not.toHaveAttribute("disabled", "", { timeout: 10000 }); + await expect( + page.locator('[data-testid="notification-service-unset"]'), + ).toHaveCount(0); }); }); }); diff --git a/tests/unit/specs/courierPushService.test.js b/tests/unit/specs/courierPushService.test.js index bf4bb515..4950ca93 100644 --- a/tests/unit/specs/courierPushService.test.js +++ b/tests/unit/specs/courierPushService.test.js @@ -1,6 +1,7 @@ import { describe, it, mock, beforeEach, afterEach } from "node:test"; import assert from "node:assert/strict"; import { CourierPushService } from "/js/push/courierPushService.js"; +import { effect } from "/js/signals.js"; const SUBSCRIPTION = { endpoint: "https://push.example/ep/abc", @@ -161,11 +162,11 @@ describe("CourierPushService heartbeat", () => { it("registers against the selected service, not the default", async () => { setupDom(); - const { service, registerPush } = createService(); globalThis.localStorage.setItem( "courier-push-service-did", "did:web:elsewhere.example", ); + const { service, registerPush } = createService(); await service._heartbeat(); assert.equal( registerPush.mock.calls[0].arguments[0].serviceDid, @@ -307,9 +308,9 @@ describe("CourierPushService service selection", () => { }); it("switching services unregisters the old one first", async () => { + globalThis.localStorage.setItem("courier-push-enabled", "true"); const { service } = createService(); await service.selectService("did:web:notifs.example"); - globalThis.localStorage.setItem("courier-push-enabled", "true"); const unregisterPush = mock.fn(async () => {}); service.api.unregisterPush = unregisterPush; @@ -322,6 +323,44 @@ describe("CourierPushService service selection", () => { assert.equal(service.isEnabled, false); }); + it("clearing the service unregisters it first", async () => { + globalThis.localStorage.setItem("courier-push-enabled", "true"); + const { service } = createService(); + await service.selectService("did:web:notifs.example"); + const unregisterPush = mock.fn(async () => {}); + service.api.unregisterPush = unregisterPush; + + await service.clearService(); + + // Same reason as switching: the old service polls server-side, so it + // must be unregistered before nothing points at it any more. + assert.equal(unregisterPush.mock.calls.length, 1); + assert.equal(service.serviceDid, null); + assert.equal(service.hasService, false); + assert.equal(service.isEnabled, false); + }); + + // Views read the service directly rather than re-reading storage when they + // are navigated back to, so its state has to notify on change. + it("notifies reactive readers when the service changes", async () => { + const { service } = createService(); + const seen = []; + // Effects flush on an animation frame, so each change needs one to land. + const flush = () => + new Promise((resolve) => requestAnimationFrame(() => resolve())); + const dispose = effect(() => { + seen.push(service.serviceDid); + }); + + await service.selectService("did:web:notifs.example"); + await flush(); + await service.clearService(); + await flush(); + + assert.deepEqual(seen, [null, "did:web:notifs.example", null]); + dispose(); + }); + it("switching services replaces the stored choice", async () => { const { service } = createService(); await service.selectService("did:web:notifs.example"); -- 2.51.2