diff --git a/docs/architecture/updates.md b/docs/architecture/updates.md index 6196d046e2..f872a7ffde 100644 --- a/docs/architecture/updates.md +++ b/docs/architecture/updates.md @@ -12,17 +12,17 @@ How to cut a release is in [`surfsense_local/RELEASE.md`](../../surfsense_local/ Updates are a call the app makes on its own, so they are off until the user turns them on. -- A new install asks in onboarding's Privacy choices step, beside usage statistics, with neither answer chosen for the user ([usage statistics](usage-statistics.md#consent)). An install that finished onboarding earlier keeps the setting it had. +- A new install asks in onboarding's first step, Privacy choices, beside usage statistics, with neither answer chosen for the user ([usage statistics](usage-statistics.md#consent)). An install that finished onboarding earlier keeps the setting it had. - The preference is `updates.json` in Electron's user data: `automatic`, false by default; `lastCheckedAt`, which Settings › Network shows as the App updates row's last call; and `lastAutomaticDay`, the UTC day of the last automatic check. - The automatic check runs at most once a UTC day: at launch, and from an hourly tick once the day turns while the app stays open. The day is marked only once a check reads a manifest, from either host. A check that reads none is tried again on the hourly tick, at most three times a UTC day. A day's automatic checks therefore approximate the installs open that day, with no identifier on the wire; a retry after a failed check can add a second event from one install. -- Check now in Settings › About, the sidebar's update row and Check for Updates… in the macOS app menu share one check (`useCheckForUpdates`), which asks first while `automatic` is off, naming the host it asks. The menu shows only that prompt; the sidebar and Settings show the progress. Allow turns `automatic` on, which also enables the daily check. +- Check now in Settings › About and Check for Updates… in the macOS app menu share one check (`useCheckForUpdates`), which asks first while `automatic` is off, naming the host it asks. Allow turns `automatic` on, which also enables the daily check. Settings shows a check's progress and result. A check from the menu, with Settings perhaps closed, reports up to date or failed in a toast; a found update shows in the title bar. +- The title bar shows updates only while one is downloading or ready, as a small pill beside the right-panel toggle. While downloading, a ring around its arrow fills with the percentage electron-updater reports, sent to the window once per whole percent; with no size known yet, part of the ring turns. Once ready, the pill fills and restarts into the update on click. On hover or focus it widens to show its label, "Downloading 64%" or "Update", with no tooltip. It shows nothing for checking, up to date or a failed check. - An admin policy file with `"updates": "off"` turns both off and locks the setting; Check now is disabled and says why. `"updateFeed"` names an `https` mirror instead of `updates.surfsense.com`, with no GitHub fallback ([usage statistics](usage-statistics.md#policy) says where the file lives). - An install a package manager owns never checks: Scoop, the AUR and Nix, whose packages write `resources/install-channel`, a snap or a flatpak, known from their environment, and a deb from SurfSense's apt repository, known from `/etc/apt/sources.list.d/surfsense.list` ([contract 5](../contracts/05-desktop-telemetry.md), part C, has the order). The setting is locked as under the policy, which names the lock when both apply. Settings › About then shows only "Updates come from" that package manager, with no daily check described and no "Show exactly what's sent". The app does not construct electron-updater at all for such an install, so a repackaged deb's `package-type` cannot start its deb updater. -- The sidebar's update row is gone while either lock holds, since a check it started would be refused. - Scoop and the AUR must first be submitted with the first release that carries `install-channel/`, not earlier: an older release has no channel lock, so it would update itself out from under them ([packaging](../../surfsense_local/packaging/README.md)). - Homebrew and winget run the release's own installer, so nothing marks them and the in-app updater stays available beside `brew upgrade` and `winget upgrade`. The Homebrew cask declares `auto_updates` inside `on_macos` only, so plain `brew upgrade` updates the Mac app while its bundle's version is older than the cask's, as it is whenever the in-app updater is off, and always updates the Linux AppImage, which has no bundle version to compare. - `autoDownload` is false. The download starts from the `update-available` event, so it only ever follows a check the user enabled or clicked. -- A downloaded update installs when the user restarts from the prompt in Settings, the sidebar or the title bar, which calls `quitAndInstall()` once the download is ready, or else the next time they quit: `autoInstallOnAppQuit`, electron-updater's default, is set to true on purpose. The update prompt says both. +- A downloaded update installs when the user restarts from Settings or the title bar's pill, which calls `quitAndInstall()` once the download is ready, or else the next time they quit: `autoInstallOnAppQuit`, electron-updater's default, is set to true on purpose. The update prompt says both. ## What a check sends @@ -45,7 +45,7 @@ Settings › About's "Show exactly what's sent" shows the last request to the fe This is not an `egress.require()` destination, because electron-updater runs in the Electron main process where the backend's check cannot see it ([egress](egress.md)). A development build shows the same controls and never checks. -An inactive electron-updater, such as its AppImage updater in an AppImage run without `$APPIMAGE`, answers a check with null and emits nothing. The check then puts back the state it found, idle unless an update was already downloaded, so Settings and the sidebar never stay on "Checking…". A check that throws keeps the `error` state electron-updater emitted first. +An inactive electron-updater, such as its AppImage updater in an AppImage run without `$APPIMAGE`, answers a check with null and emits nothing. The check then puts back the state it found, idle unless an update was already downloaded, so Settings never stays on "Checking…". A check that throws keeps the `error` state electron-updater emitted first. ## Update size diff --git a/docs/architecture/usage-statistics.md b/docs/architecture/usage-statistics.md index 9acbdc6b28..95b650ce93 100644 --- a/docs/architecture/usage-statistics.md +++ b/docs/architecture/usage-statistics.md @@ -32,9 +32,9 @@ No identifier ties one install's reports together, but the values that never cha ## Consent -- A new install answers in the Privacy choices step, right after the welcome in onboarding: two rows, update checks and usage statistics, each with equal Yes and No buttons, neither chosen beforehand, and Continue only once both are answered. Continue calls `completeFirstRun`, which writes both answers and `firstRunAsked`. Each row opens exactly what it sends. -- An install that finished onboarding never sees the step, and one that answered (`firstRunAsked`) skips it. Its usage statistics stay `null`, read as off, until the user turns them on. A bridge that cannot read its state skips the step too, with both left off, rather than holding setup on an empty card. -- Settings › About has the Usage statistics switch beside App updates, with "Show exactly what's sent": the next report as Electron main would build it now (`previewUsageReport`, not sent), the last report sent and when, and the host. Settings › Network lists the same switch as a row with its host and its last call. +- Privacy choices is onboarding's first step, right after the welcome, counted in its progress like the model steps: two rows, update checks and usage statistics, each with equal Yes and No buttons, neither chosen beforehand, and Continue only once both are answered. Continue calls `completeFirstRun`, which writes both answers and `firstRunAsked`. Each row says in one line what it does; the info icon beside its title shows the full description with every host its answer reaches, and the link below opens exactly what it sends. +- Onboarding always shows the step in the desktop app. An install that answered already (`firstRunAsked`), or that comes back from the first model step, sees its answers chosen. An install that finished onboarding never sees it; its usage statistics stay `null`, read as off, until the user turns them on. A bridge that cannot read its state skips the step, with both left off, rather than holding setup on an empty card. +- Settings › About has the Usage statistics switch beside App updates. Both say what they do as onboarding does, in the same one-line summary with the full description behind an info icon, and each has "Show exactly what's sent". For usage statistics that shows the next report as Electron main would build it now (`previewUsageReport`, not sent), the last report sent and when, and the host. Settings › Network lists the same switch as a row with its host and its last call. - Turning the switch off stops the next report; nothing already sent can be recalled, because no identifier marks which reports were this install's. ## Policy diff --git a/surfsense_local/electron/src/main/updater.test.ts b/surfsense_local/electron/src/main/updater.test.ts index 2ac8b1a3cd..a4e1b667c4 100644 --- a/surfsense_local/electron/src/main/updater.test.ts +++ b/surfsense_local/electron/src/main/updater.test.ts @@ -64,7 +64,7 @@ test("a found update is downloaded and offered, and waits for a restart or a qui assert.deepEqual(fake.calls, ["check", "download"]) assert.deepEqual(states, [ { status: "checking" }, - { status: "downloading", version: "1.0.1" }, + { status: "downloading", version: "1.0.1", percent: null }, { status: "ready", version: "1.0.1" }, ]) assert.deepEqual(updates.state(), { status: "ready", version: "1.0.1" }) @@ -73,6 +73,37 @@ test("a found update is downloaded and offered, and waits for a restart or a qui assert.deepEqual(fake.calls, ["check", "download", "install"]) }) +test("a download reports its progress in whole percents, once per step", async () => { + const fake = fakeAutoUpdater() + const states: UpdateState[] = [] + const updates = attachUpdater(fake.updater, (state) => states.push(state), FEED) + + await updates.check() + fake.emit("update-available", { version: "1.0.1" }) + for (const percent of [0.4, 12.2, 12.9, 64.5, 100]) + fake.emit("download-progress", { percent, total: 1000 }) + // No size known yet, so no percentage to show. + fake.emit("download-progress", { percent: 0, total: 0 }) + + assert.deepEqual( + states.slice(1).map((state) => (state.status === "downloading" ? state.percent : state.status)), + [null, 0, 12, 64, 100, null] + ) + assert.deepEqual(updates.state(), { status: "downloading", version: "1.0.1", percent: null }) +}) + +test("progress after the download finished leaves it ready", async () => { + const fake = fakeAutoUpdater() + const updates = attachUpdater(fake.updater, () => undefined, FEED) + + await updates.check() + fake.emit("update-available", { version: "1.0.1" }) + fake.emit("update-downloaded", { version: "1.0.1" }) + fake.emit("download-progress", { percent: 100, total: 1000 }) + + assert.deepEqual(updates.state(), { status: "ready", version: "1.0.1" }) +}) + test("checks go through SurfSense's feed provider, with no headers left for downloads", () => { const fake = fakeAutoUpdater() fake.updater.requestHeaders = { "x-user-staging-id": "0" } diff --git a/surfsense_local/electron/src/main/updater.ts b/surfsense_local/electron/src/main/updater.ts index 7efefbf488..62c08cdd68 100644 --- a/surfsense_local/electron/src/main/updater.ts +++ b/surfsense_local/electron/src/main/updater.ts @@ -24,7 +24,8 @@ export type UpdateState = | { status: "idle" } | { status: "checking" } | { status: "up-to-date" } - | { status: "downloading"; version: string } + // percent: whole percents, null until the download's size is known. + | { status: "downloading"; version: string; percent: number | null } | { status: "ready"; version: string } | { status: "error"; message: string } @@ -104,9 +105,16 @@ export function attachUpdater( // `updater.channel`, whose setter turns on allowDowngrade. updater.setFeedURL(surfSenseFeedOptions(feed)) updater.on("update-available", (info: { version: string }) => { - set({ status: "downloading", version: info.version }) + set({ status: "downloading", version: info.version, percent: null }) updater.downloadUpdate().catch(() => undefined) }) + // Sent once per whole percent, so the window is not redrawn many times a second. + updater.on("download-progress", (progress: { percent: number; total: number }) => { + if (current.status !== "downloading") return + const percent = + progress.total > 0 ? Math.min(100, Math.max(0, Math.floor(progress.percent))) : null + if (percent !== current.percent) set({ ...current, percent }) + }) updater.on("update-downloaded", (info: { version: string }) => { set({ status: "ready", version: info.version }) }) diff --git a/surfsense_local/frontend/src/components/ui/icons.tsx b/surfsense_local/frontend/src/components/ui/icons.tsx index da8b2ba1cd..25b4458ab4 100644 --- a/surfsense_local/frontend/src/components/ui/icons.tsx +++ b/surfsense_local/frontend/src/components/ui/icons.tsx @@ -6,6 +6,7 @@ import { AiImageEditIcon as AiImageEditIconData, AiSearchLinesIcon as AiSearchLinesIconData, ArrowDownIcon as ArrowDownIconData, + ArrowDown02Icon as ArrowDown02IconData, ArrowExpand01Icon as ArrowExpand01IconData, ArrowLeftIcon as ArrowLeftIconData, ArrowRightIcon as ArrowRightIconData, @@ -135,6 +136,7 @@ export const AiEditingIcon = createIcon(AiEditingIconData) export const AiImageEditIcon = createIcon(AiImageEditIconData) export const AiSearchLinesIcon = createIcon(AiSearchLinesIconData) export const ArrowDownIcon = createIcon(ArrowDownIconData) +export const ArrowDown02Icon = createIcon(ArrowDown02IconData) export const ArrowExpand01Icon = createIcon(ArrowExpand01IconData) export const ArrowLeftIcon = createIcon(ArrowLeftIconData) export const ArrowRightIcon = createIcon(ArrowRightIconData) diff --git a/surfsense_local/frontend/src/features/about/data-folder.tsx b/surfsense_local/frontend/src/features/about/data-folder.tsx index 3742fc1259..220a8a8bc4 100644 --- a/surfsense_local/frontend/src/features/about/data-folder.tsx +++ b/surfsense_local/frontend/src/features/about/data-folder.tsx @@ -32,29 +32,29 @@ export function DataFolder() { } return ( -
-

- {intl.formatMessage({ - id: "about_data_folder_title", - defaultMessage: "Your data", - })} -

-

- {intl.formatMessage({ - id: "about_data_folder_body", - defaultMessage: - "Models, workspaces and settings are kept here, not in the folder SurfSense is installed in.", - })} -

- {folder} -
- + +

+ {intl.formatMessage({ + id: "about_data_folder_body", + defaultMessage: + "Models, workspaces and settings are kept here, not in the folder SurfSense is installed in.", + })} +

+ {folder}
+
) } diff --git a/surfsense_local/frontend/src/features/dashboard/dashboard-page.tsx b/surfsense_local/frontend/src/features/dashboard/dashboard-page.tsx index 07dc79b842..66410a7bed 100644 --- a/surfsense_local/frontend/src/features/dashboard/dashboard-page.tsx +++ b/surfsense_local/frontend/src/features/dashboard/dashboard-page.tsx @@ -60,7 +60,7 @@ import { } from "@/features/office-support/office-offer" import { StudioPanel } from "@/features/studio/studio-panel" import { useStudio } from "@/features/studio/use-studio" -import { UpdateButton } from "@/features/updates/update-settings" +import { TitleBarUpdate } from "@/features/updates/title-bar-update" import type { Workspace } from "@/features/workspaces/api" import { useWorkspaces } from "@/features/workspaces/use-workspaces" import { useStableCallback } from "@/hooks/use-stable-callback" @@ -385,7 +385,7 @@ function WorkspaceDashboard({
- + void = vi.fn()) { - return render( - <> - - - - ) + return render() } afterEach(() => { @@ -93,130 +84,27 @@ describe("the license row", () => { }) }) -describe("the update row", () => { - it("checks on demand, then offers the restart once one is ready", async () => { +describe("updates", () => { + it("are not in the footer: the title bar shows them", async () => { stubLicense(license("active")) - const bridge = stubUpdateBridge({ + stubUpdateBridge({ automatic: true, - state: { status: "idle" }, - }) - const user = userEvent.setup() - renderFooter() - - await user.click( - await screen.findByRole("button", { name: "Check for updates" }) - ) - expect(bridge.calls).toEqual(["check"]) - - bridge.push({ status: "downloading", version: "1.0.1" }) - const downloading = await screen.findByRole("button", { - name: "Downloading…", - }) - // Clicking mid-download would only start a second check. - expect((downloading as HTMLButtonElement).disabled).toBe(true) - - bridge.push({ status: "ready", version: "1.0.1" }) - await user.click( - await screen.findByRole("button", { name: "Restart to update" }) - ) - - expect(bridge.calls).toEqual(["check", "install"]) - }) - - it("asks before the first check, and checks once allowed", async () => { - stubLicense(license("active")) - const bridge = stubUpdateBridge({ - automatic: false, - state: { status: "idle" }, - }) - const user = userEvent.setup() - renderFooter() - - await user.click( - await screen.findByRole("button", { name: "Check for updates" }) - ) - - // Named like any other destination, because that is what it is. - await screen.findByRole("alertdialog", { - name: "Allow SurfSense to check for updates?", - }) - expect(bridge.calls).toEqual([]) - - await user.click(screen.getByRole("button", { name: "Allow" })) - - // Allowing is what Settings > Network's switch does, then the call runs. - await waitFor(() => - expect(bridge.calls).toEqual(["automatic:true", "check"]) - ) - }) - - it("cancelling leaves github.com alone", async () => { - stubLicense(license("active")) - const bridge = stubUpdateBridge({ - automatic: false, - state: { status: "idle" }, - }) - const user = userEvent.setup() - renderFooter() - - await user.click( - await screen.findByRole("button", { name: "Check for updates" }) - ) - await screen.findByRole("alertdialog") - await user.click(screen.getByRole("button", { name: "Cancel" })) - - await waitFor(() => expect(screen.queryByRole("alertdialog")).toBeNull()) - expect(bridge.calls).toEqual([]) - }) - - it("installs an update already downloaded without asking", async () => { - stubLicense(license("active")) - const bridge = stubUpdateBridge({ - automatic: false, state: { status: "ready", version: "1.0.1" }, }) - const user = userEvent.setup() renderFooter() - // Installing touches no network, so it needs no permission even though - // checking for this update would. - await user.click( - await screen.findByRole("button", { name: "Restart to update" }) - ) - - expect(bridge.calls).toEqual(["install"]) - expect(screen.queryByRole("alertdialog")).toBeNull() + await screen.findByRole("button", { name: "License active" }) + expect(screen.queryByRole("button", { name: /update/i })).toBeNull() }) - it.each([ - [ - "a package manager owns updates", - { lockedReason: "channel", channel: "snap" }, - ], - ["a policy turned them off", { lockedReason: "policy", channel: null }], - ] as const)("is gone while %s", async (_, lock) => { - stubLicense(license("active")) - const bridge = stubPrivacyBridge({ updates: { automatic: true } }) - renderFooter() - - // The privacy state is read, and leaves the row while nothing locks it. - await screen.findByRole("button", { name: "Check for updates" }) - bridge.push({ updates: { locked: true, ...lock } }) - - // A check the app refuses would make the row a button that does nothing. - await waitFor(() => - expect(screen.queryByRole("button", { name: /update/i })).toBeNull() + it("leave no footer at all when there is no license status", () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Promise(() => undefined)) ) - expect(screen.getByRole("button", { name: "License active" })).toBeTruthy() - }) - - it("renders no update row outside the desktop app", async () => { - stubLicense(license("active")) - renderFooter() - - // The license still reports; only the bridge-backed row is missing. - await screen.findByRole("button", { name: "License active" }) + stubUpdateBridge({ automatic: true, state: { status: "idle" } }) + const { container } = renderFooter() - expect(screen.queryByRole("button", { name: /update/i })).toBeNull() + expect(container.querySelector(".border-t")).toBeNull() }) }) diff --git a/surfsense_local/frontend/src/features/dashboard/sidebar-footer.tsx b/surfsense_local/frontend/src/features/dashboard/sidebar-footer.tsx index b35b334dc1..9f685dd64c 100644 --- a/surfsense_local/frontend/src/features/dashboard/sidebar-footer.tsx +++ b/surfsense_local/frontend/src/features/dashboard/sidebar-footer.tsx @@ -1,26 +1,16 @@ import type { ComponentType } from "react" import { Button } from "@/components/ui/button" -import { DownloadCircle02Icon, LicenseIcon } from "@/components/ui/icons" +import { LicenseIcon } from "@/components/ui/icons" import type { LicenseState } from "@/features/license/api" import { useLicense } from "@/features/license/use-license" -import { usePrivacyState } from "@/features/privacy-choices/use-privacy-state" -import { useCheckForUpdates } from "@/features/updates/use-check-for-updates" -import { - updatesBridge, - useUpdateState, -} from "@/features/updates/use-update-state" import { intl } from "@/i18n/intl" -import type { UpdateState } from "@/lib/api" import { cn } from "@/lib/utils" // `good` and `bad` keep their colour on hover, where ghost would otherwise // repaint them: a status light that changes colour under the pointer is no // longer reporting anything. const TONE = { - quiet: "text-muted-foreground", - offer: "text-notice hover:text-notice", - wrong: "text-amber-600 dark:text-amber-500", good: "text-emerald-600 hover:text-emerald-600 dark:text-emerald-500 dark:hover:text-emerald-500", bad: "text-destructive hover:text-destructive", } as const @@ -28,7 +18,6 @@ const TONE = { type Row = { label: string tone: keyof typeof TONE - busy?: boolean } function FooterRow({ @@ -46,7 +35,6 @@ function FooterRow({ type="button" variant="ghost" size="xs" - disabled={row.busy} className={cn("w-full justify-start px-2 font-normal", TONE[row.tone])} onClick={onClick} > @@ -104,68 +92,12 @@ function licenseRow(state: LicenseState): Row { return { label: label(), tone } } -function updateRow(state: UpdateState): Row { - switch (state.status) { - case "checking": - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_checking_status", - defaultMessage: "Checking…", - }), - tone: "quiet", - busy: true, - } - case "downloading": - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_downloading_status", - defaultMessage: "Downloading…", - }), - tone: "quiet", - busy: true, - } - case "ready": - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_restart_button", - defaultMessage: "Restart to update", - }), - tone: "offer", - } - case "up-to-date": - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_current_status", - defaultMessage: "Up to date", - }), - tone: "quiet", - } - case "error": - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_failed_status", - defaultMessage: "Update check failed", - }), - tone: "wrong", - } - default: - return { - label: intl.formatMessage({ - id: "dashboard_footer_update_check_button", - defaultMessage: "Check for updates", - }), - tone: "quiet", - } - } -} - /** - * The sidebar's bottom edge, holding only what is worth a glance: where the - * license stands, and where the app stands on updates. + * The sidebar's bottom edge: where the license stands, at a glance. Updates + * show in the title bar instead. * - * Either row can be absent -- no update row outside the desktop app or while - * updates are locked, no license row until its status arrives -- so the strip - * goes with them rather than leaving a bordered gap. + * Absent until the license status arrives, so the strip never leaves a + * bordered gap. */ export function SidebarFooter({ onOpenLicense, @@ -173,45 +105,15 @@ export function SidebarFooter({ onOpenLicense: () => void }) { const license = useLicense().data - const updates = updatesBridge() - const state = useUpdateState() - const checkForUpdates = useCheckForUpdates() - const privacy = usePrivacyState() - - const currentLicenseRow = license ? licenseRow(license.state) : null - // No row while a policy or a package manager locks updates, where a click - // would do nothing, nor until the privacy state says whether one does. - const showUpdates = - Boolean(updates) && - (privacy.state === null || - privacy.failed || - privacy.state?.updates.locked === false) - - if (!currentLicenseRow && !showUpdates) return null - - // Installing is local and needs no permission; checking goes through consent. - const onUpdateClick = async () => { - if (!updates) return - if (state.status === "ready") return void updates.install() - await checkForUpdates() - } + if (!license) return null return (
- {currentLicenseRow ? ( - - ) : null} - {showUpdates ? ( - void onUpdateClick()} - /> - ) : null} +
) } diff --git a/surfsense_local/frontend/src/features/onboarding/onboarding-page.test.tsx b/surfsense_local/frontend/src/features/onboarding/onboarding-page.test.tsx index 7e21df4d00..f9f2a50556 100644 --- a/surfsense_local/frontend/src/features/onboarding/onboarding-page.test.tsx +++ b/surfsense_local/frontend/src/features/onboarding/onboarding-page.test.tsx @@ -1342,6 +1342,16 @@ describe("the privacy choices", () => { await screen.findByRole("heading", { name: "Privacy choices" }) } + // The full description is the title's tooltip. + async function hoverDetails( + user: ReturnType, + label: string + ) { + await user.hover(screen.getByRole("button", { name: label })) + const tooltip = await screen.findByRole("tooltip") + return tooltip.textContent ?? "" + } + function choice(row: string, answer: "Yes" | "No") { return within(screen.getByRole("group", { name: row })).getByRole( "button", @@ -1356,8 +1366,9 @@ describe("the privacy choices", () => { render( undefined} />) await toPrivacyStep(user) - // Not a model step: the progress bar starts after it. - expect(screen.queryByLabelText(/Onboarding step/)).toBeNull() + // The first step like any other: counted, with no way back to the welcome. + expect(screen.getByLabelText("Onboarding step 1 of 7")).toBeTruthy() + expect(screen.queryByRole("button", { name: "Back" })).toBeNull() for (const row of ["Check for updates", "Share usage statistics"]) for (const answer of ["Yes", "No"] as const) expect(choice(row, answer).getAttribute("aria-pressed")).toBe("false") @@ -1387,17 +1398,32 @@ describe("the privacy choices", () => { render( undefined} />) await toPrivacyStep(user) + // One line each; the hosts are in the info icon's description. const updates = screen.getByRole("region", { name: "Check for updates" }) - expect(updates.textContent).toContain("updates.surfsense.com") - expect(updates.textContent).toContain("github.com") - expect(updates.textContent).toContain("no ID") - // The download and where it comes from, as the update prompt says. - expect(updates.textContent).toContain("your IP address") - expect(updates.textContent).toContain("githubusercontent.com") - expect(updates.textContent).toContain("the next time you quit") + expect(updates.textContent).toContain("No ID sent.") + expect(updates.textContent).not.toContain("updates.surfsense.com") const usage = screen.getByRole("region", { name: "Share usage statistics" }) - expect(usage.textContent).toContain("usage.surfsense.com") expect(usage.textContent).toContain( + "Never your documents, prompts or an ID." + ) + + // Every host, and the download, as the update prompt says. + const updatesDetails = await hoverDetails( + user, + "What an update check sends" + ) + expect(updatesDetails).toContain("updates.surfsense.com") + expect(updatesDetails).toContain("github.com") + expect(updatesDetails).toContain("your IP address") + expect(updatesDetails).toContain("githubusercontent.com") + expect(updatesDetails).toContain("the next time you quit") + await user.unhover( + screen.getByRole("button", { name: "What an update check sends" }) + ) + await waitFor(() => expect(screen.queryByRole("tooltip")).toBeNull()) + const usageDetails = await hoverDetails(user, "What usage statistics send") + expect(usageDetails).toContain("usage.surfsense.com") + expect(usageDetails).toContain( "Never documents, prompts, file names, model names, hosts, your license key or an ID." ) }) @@ -1411,7 +1437,7 @@ describe("the privacy choices", () => { await toPrivacyStep(user) const usage = screen.getByRole("region", { name: "Share usage statistics" }) await user.click( - within(usage).getByRole("button", { name: "See exactly what’s sent" }) + within(usage).getByRole("button", { name: "Show exactly what’s sent" }) ) const dialog = await screen.findByRole("dialog", { @@ -1433,7 +1459,7 @@ describe("the privacy choices", () => { await toPrivacyStep(user) const updates = screen.getByRole("region", { name: "Check for updates" }) await user.click( - within(updates).getByRole("button", { name: "See exactly what’s sent" }) + within(updates).getByRole("button", { name: "Show exactly what’s sent" }) ) const dialog = await screen.findByRole("dialog", { @@ -1490,7 +1516,14 @@ describe("the privacy choices", () => { expect(updates.textContent).toContain("Updates come from the Snap Store") expect(updates.textContent).not.toContain("Once a day") expect( - within(updates).queryByRole("button", { name: "See exactly what’s sent" }) + within(updates).queryByRole("button", { + name: "Show exactly what’s sent", + }) + ).toBeNull() + expect( + within(updates).queryByRole("button", { + name: "What an update check sends", + }) ).toBeNull() expect( screen.queryByRole("group", { name: "Check for updates" }) @@ -1521,16 +1554,50 @@ describe("the privacy choices", () => { ).toBeNull() }) - it("never asks an install that already answered", async () => { + it("asks an install that already answered again, with its answers chosen", async () => { vi.stubGlobal("fetch", backend()) - stubPrivacyBridge({ firstRunAsked: true }) + stubPrivacyBridge({ + firstRunAsked: true, + updates: { automatic: true }, + usageStatistics: { enabled: false }, + }) const user = userEvent.setup() render( undefined} />) - // Whether or not its answer was read before the welcome was left. - await toChatStep(user) + await toPrivacyStep(user) expect( - screen.queryByRole("heading", { name: "Privacy choices" }) - ).toBeNull() + choice("Check for updates", "Yes").getAttribute("aria-pressed") + ).toBe("true") + expect( + choice("Share usage statistics", "No").getAttribute("aria-pressed") + ).toBe("true") + expect( + screen.getByRole("button", { name: "Continue" }).hasAttribute("disabled") + ).toBe(false) + }) + + it("goes back to the choices from the first model step, keeping them", async () => { + vi.stubGlobal("fetch", backend({ selections: { ...chatChosen } })) + stubPrivacyBridge() + const user = userEvent.setup() + render( undefined} />) + + await toPrivacyStep(user) + await user.click(choice("Check for updates", "No")) + await user.click(choice("Share usage statistics", "Yes")) + await user.click(screen.getByRole("button", { name: "Continue" })) + await screen.findByRole("heading", { + name: "Choose a text generation model", + }) + expect(screen.getByLabelText("Onboarding step 2 of 7")).toBeTruthy() + + await user.click(screen.getByRole("button", { name: "Back" })) + await screen.findByRole("heading", { name: "Privacy choices" }) + expect(choice("Check for updates", "No").getAttribute("aria-pressed")).toBe( + "true" + ) + expect( + choice("Share usage statistics", "Yes").getAttribute("aria-pressed") + ).toBe("true") }) }) diff --git a/surfsense_local/frontend/src/features/onboarding/onboarding-page.tsx b/surfsense_local/frontend/src/features/onboarding/onboarding-page.tsx index a32213034f..b93054b7d5 100644 --- a/surfsense_local/frontend/src/features/onboarding/onboarding-page.tsx +++ b/surfsense_local/frontend/src/features/onboarding/onboarding-page.tsx @@ -16,14 +16,15 @@ import { PrivacyStep } from "./privacy-step/privacy-step" import { useFinishOnboarding } from "./use-finish-onboarding" /** - * The steps, in order, and the ones the progress bar counts. The welcome is still - * onboarding, and still gated by the same marker, but it is an introduction, - * not a step to complete. Whichever step is last finishes onboarding. + * The model steps, in order. The privacy choices come before them as the first + * step the progress bar counts. The welcome is still onboarding, and still gated + * by the same marker, but it is an introduction, not a step to complete. + * Whichever step is last finishes onboarding. * * The embedding model comes last and is the one step that is not a slot: its * choice is sent with the finish that fixes it, never held across steps. */ -const STEPS = [ +const MODEL_STEPS = [ "text_gen", "image_gen", "image_edit", @@ -32,13 +33,12 @@ const STEPS = [ "embedding", ] as const satisfies readonly OnboardingStepKind[] -type Step = (typeof STEPS)[number] -// The privacy choices sit between the welcome and the steps, and like the -// welcome they are not a step the progress bar counts. -type Screen = "welcome" | "privacy" | Step +type ModelStepKind = (typeof MODEL_STEPS)[number] +type Step = "privacy" | ModelStepKind +type Screen = "welcome" | Step /** Finishing needs a chat model, so only this step cannot be skipped. */ -const REQUIRED_STEP: Step = "text_gen" +const REQUIRED_STEP: ModelStepKind = "text_gen" function OnboardingBrand() { return ( @@ -60,8 +60,14 @@ function OnboardingBrand() { ) } -function OnboardingProgress({ screen }: { screen: Step }) { - const step = STEPS.indexOf(screen) + 1 +function OnboardingProgress({ + screen, + steps, +}: { + screen: Step + steps: readonly Step[] +}) { + const step = steps.indexOf(screen) + 1 const label = intl.formatMessage( { id: "onboarding_progress_aria", @@ -69,13 +75,13 @@ function OnboardingProgress({ screen }: { screen: Step }) { }, { step, - total: STEPS.length, + total: steps.length, } ) return ( label} className="mx-auto w-full max-w-28 **:data-[slot=progress-indicator]:rounded-full **:data-[slot=progress-indicator]:duration-300 **:data-[slot=progress-indicator]:ease-out **:data-[slot=progress-track]:bg-border **:data-[slot=progress-indicator]:motion-reduce:transition-none" @@ -172,20 +178,22 @@ function WelcomeStep({ onNext }: { onNext: () => void }) { ) } -/** One step in the order of `STEPS`: the last one finishes, the rest move on. */ +/** One model step in the order of `steps`: the last one finishes, the rest move on. */ function OnboardingStep({ step, + steps, onGo, onComplete, }: { - step: Step + step: ModelStepKind + steps: readonly Step[] onGo: (step: Step) => void onComplete: (selection: ModelSelection) => void }) { const { finish, finishing, error } = useFinishOnboarding(onComplete) - const index = STEPS.indexOf(step) - const previous = STEPS[index - 1] - const next = STEPS[index + 1] + const index = steps.indexOf(step) + const previous = steps[index - 1] + const next = steps[index + 1] // The last step finishes, sending its value: the embedding choice, or null. const advance = (value: string | null) => next ? onGo(next) : void finish(value) @@ -209,14 +217,16 @@ export function OnboardingPage({ }) { const [chosen, setScreen] = useState("welcome") const { state: privacy, failed: privacyUnreadable } = usePrivacyState() - // Asked once, on a new install: never outside the desktop app, or once - // answered. An unreadable state skips it, leaving both off, rather than - // holding setup on an empty card. - const privacyAnswered = - privacy === null || privacy?.firstRunAsked === true || privacyUnreadable - // Left the welcome before the answer was read, and none was needed. + // Always asked in the desktop app. Outside it there is nothing to save to, + // and an unreadable state skips it, leaving both off, rather than holding + // setup on an empty card. + const asksPrivacy = privacy !== null && !privacyUnreadable + const steps: readonly Step[] = asksPrivacy + ? ["privacy", ...MODEL_STEPS] + : MODEL_STEPS + // Left the welcome before the state was read, and it could not be. const screen: Screen = - chosen === "privacy" && privacyAnswered ? STEPS[0] : chosen + chosen === "privacy" && !asksPrivacy ? MODEL_STEPS[0] : chosen return (
- {screen === "welcome" || screen === "privacy" ? ( + {screen === "welcome" ? ( // The progress bar's height, kept so the hero sits exactly where it did.