From 391ea143f2f0ba9b2b7fe36cef352ab258e58f22 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 02:05:54 -0400 Subject: [PATCH] Revert "fix(settings): say how many computers Turn on would reach" This reverts commit 42a4320ae1. The roll-up row's design is still open, so the branch keeps the summary sentence and the plain Turn on all button until it is settled. The dialog removal in 30b0786cc6 stands. --- .../SessionHistorySettingsPane.test.tsx | 50 ++++++---------- .../settings/SessionHistorySettingsPane.tsx | 26 ++++----- .../session-search-computer-rollup.test.ts | 43 ++++++++++---- .../session-search-computer-rollup.ts | 57 +++++++++++++++++-- src/renderer/src/i18n/locales/en.json | 9 ++- 5 files changed, 118 insertions(+), 67 deletions(-) diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx index 5975d946f7d..270b651e24d 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx @@ -110,7 +110,7 @@ function statusByHost(): void { return answer }) } -const turnOnButton = (name: string | RegExp) => screen.queryByRole('button', { name }) +const summaryLine = (): string => screen.getByText(/computers/).textContent ?? '' async function openAdvanced(): Promise { await act(async () => { fireEvent.click(screen.getByRole('button', { name: /Advanced/ })) @@ -359,7 +359,7 @@ it('offers only this computer to a paired client, with no server rows', async () expect(screen.getAllByRole('switch')).toHaveLength(1) expect(screen.getByRole('switch')).toBeDisabled() expect(screen.queryByRole('status')).not.toBeInTheDocument() - expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Open' })).not.toBeInTheDocument() expect(mocks.status).not.toHaveBeenCalled() }) @@ -391,30 +391,25 @@ it('leaves a lone computer to its own switch, with no roll-up above it', async ( pane(true) await act(async () => {}) expect(screen.getAllByRole('switch')).toHaveLength(1) - expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() + expect(screen.queryByText(/of 1 computers/)).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() expect(screen.queryByText('This computer')).not.toBeInTheDocument() expect(screen.queryByText('Orca remote servers')).not.toBeInTheDocument() }) -it('names in the button how many computers it would actually turn on', async () => { +it('counts every computer in one line, leaving out the segments worth zero', async () => { mixedFleet() pane(true) await act(async () => {}) - // Of five computers only gpu-a is reachable, new enough and off; the offline and - // too-old ones are not counted, and this computer and build-01 are already on. - expect(turnOnButton('Turn on for all 1')).toBeInTheDocument() - - mocks.environments = [ - { id: 'a', name: 'gpu-a' }, - { id: 'b', name: 'gpu-b' } - ] - mocks.details = { a: CONNECTED_DETAILS, b: CONNECTED_DETAILS } - mocks.statusByHost = { local: off, 'runtime:a': off, 'runtime:b': off } + expect(summaryLine()).toBe('On 2 of 5 computers · 1 offline · 1 need an update') + mocks.environments = [{ id: 'off', name: 'gpu-a' }] + mocks.details = { off: CONNECTED_DETAILS } + mocks.statusByHost = { local: current, 'runtime:off': off } statusByHost() cleanup() - pane(false) + pane(true) await act(async () => {}) - expect(turnOnButton('Turn on for all 3')).toBeInTheDocument() + expect(summaryLine()).toBe('On 1 of 2 computers') }) it('turns on every reachable computer and skips the ones it cannot', async () => { @@ -424,7 +419,7 @@ it('turns on every reachable computer and skips the ones it cannot', async () => pane(true, confirm, save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 1' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) }) expect(confirm).not.toHaveBeenCalled() expect(mocks.setEnabled.mock.calls.map((call) => call[0])).toEqual(['runtime:off']) @@ -440,7 +435,7 @@ it('turns this computer on as part of turning them all on', async () => { pane(false, vi.fn().mockResolvedValue(true), save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 2' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) }) expect(save).toHaveBeenCalledWith({ aiVaultSearch: { enabled: true, historyDays: null } }) expect(mocks.setEnabled).toHaveBeenCalledWith('runtime:off', true) @@ -459,14 +454,14 @@ it('keeps going after a host refuses, and withholds the standing consent', async pane(true, vi.fn().mockResolvedValue(true), save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 2' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) }) expect(mocks.setEnabled.mock.calls.map((call) => call[0])).toEqual(['runtime:a', 'runtime:b']) expect(screen.getByRole('alert')).toHaveTextContent('Could not change session search on gpu-a') expect(save).not.toHaveBeenCalledWith({ aiVaultSearchAutoEnableNewComputers: true }) }) -it('replaces the button with the standing promise once nothing is left to turn on', async () => { +it('hides the button and says so once nothing is left to turn on', async () => { mocks.environments = [ { id: 'a', name: 'gpu-a' }, { id: 'gone', name: 'linux 1' } @@ -476,19 +471,8 @@ it('replaces the button with the standing promise once nothing is left to turn o statusByHost() pane(true, undefined, undefined, null, true) await act(async () => {}) - expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() - expect(screen.getByText('New computers turn on automatically.')).toBeInTheDocument() -}) - -it('stays silent about automatic turn-on when that consent was never given', async () => { - mocks.environments = [{ id: 'a', name: 'gpu-a' }] - mocks.details = { a: CONNECTED_DETAILS } - mocks.statusByHost = { local: current, 'runtime:a': current } - statusByHost() - pane(true) - await act(async () => {}) - expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() - expect(screen.queryByText('New computers turn on automatically.')).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() + expect(summaryLine()).toBe('On 2 of 3 computers · 1 offline New computers turn on when they can.') }) it('turns on a newly reachable server while the standing consent holds', async () => { diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx index 16256298985..b2ec893f0a0 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx @@ -20,9 +20,10 @@ import { SessionHistoryComputerRow } from './SessionHistoryComputerRow' import { SessionHistoryServerRow } from './SessionHistoryServerRow' import { SessionSearchComputerList } from './SessionSearchComputerList' import { - countTurnOnableSessionSearchComputers, isTurnOnableSessionSearchState, orderSessionSearchServers, + sessionSearchSummarySentence, + summarizeSessionSearchComputers, type SessionSearchComputerEntry, type SessionSearchComputerState } from './session-search-computer-rollup' @@ -81,7 +82,7 @@ export function SessionHistorySettingsPane({ state: serverStates[environment.id] ?? 'checking', environment })) - const turnOnableCount = countTurnOnableSessionSearchComputers([localEntry, ...serverEntries]) + const summary = summarizeSessionSearchComputers([localEntry, ...serverEntries]) const orderedServers = orderSessionSearchServers(serverEntries) // Rebuilt each render on purpose: the hook keys off the host ids, not this array. const autoEnableTargets = serverEntries @@ -229,11 +230,13 @@ export function SessionHistorySettingsPane({ )}

- {/* With no paired server this row only restates the single switch below it. Each row - already says whether it is offline or too old, so nothing here counts those again. */} + {/* With no paired server the line and the button only restate the single switch below them. */} {serverEntries.length === 0 ? null : ( -
- {turnOnableCount > 0 ? ( +
+

+ {sessionSearchSummarySentence(summary, autoEnableNewComputers)} +

+ {summary.turnOnable > 0 ? ( - ) : autoEnableNewComputers ? ( -

- {translate( - 'sessionHistory.settings.autoEnableArmed', - 'New computers turn on automatically.' - )} -

) : null}
)} diff --git a/src/renderer/src/components/settings/session-search-computer-rollup.test.ts b/src/renderer/src/components/settings/session-search-computer-rollup.test.ts index f2cd784ec45..66f94a1a33b 100644 --- a/src/renderer/src/components/settings/session-search-computer-rollup.test.ts +++ b/src/renderer/src/components/settings/session-search-computer-rollup.test.ts @@ -1,11 +1,17 @@ -import { expect, it } from 'vitest' +import { expect, it, vi } from 'vitest' import { - countTurnOnableSessionSearchComputers, isTurnOnableSessionSearchState, orderSessionSearchServers, + sessionSearchSummarySentence, + summarizeSessionSearchComputers, type SessionSearchComputerEntry } from './session-search-computer-rollup' +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, args?: Record) => + fallback.replace(/{{(\w+)}}/g, (_, key: string) => String(args?.[key])) +})) + const fleet: SessionSearchComputerEntry[] = [ { id: 'local', name: 'Local Mac', state: 'on' }, { id: 'a', name: 'build-01', state: 'on' }, @@ -16,15 +22,14 @@ const fleet: SessionSearchComputerEntry[] = [ { id: 'f', name: 'probing', state: 'checking' } ] -it('counts only the computers a turn-on would actually reach', () => { - expect(countTurnOnableSessionSearchComputers(fleet)).toBe(1) - expect(countTurnOnableSessionSearchComputers([])).toBe(0) - expect( - countTurnOnableSessionSearchComputers([ - { id: 'a', name: 'a', state: 'off' }, - { id: 'b', name: 'b', state: 'off' } - ]) - ).toBe(2) +it('counts what the user can see and what they could act on', () => { + expect(summarizeSessionSearchComputers(fleet)).toEqual({ + on: 2, + total: 7, + offline: 2, + needUpdate: 1, + turnOnable: 1 + }) }) it('will not offer to turn on a computer it cannot reach or that is too old', () => { @@ -34,6 +39,22 @@ it('will not offer to turn on a computer it cannot reach or that is too old', () } }) +it('leaves a zero segment out of the sentence rather than printing it', () => { + expect(sessionSearchSummarySentence(summarizeSessionSearchComputers(fleet), false)).toBe( + 'On 2 of 7 computers · 2 offline · 1 need an update' + ) + const onlyLocal = summarizeSessionSearchComputers([fleet[0]]) + expect(sessionSearchSummarySentence(onlyLocal, false)).toBe('On 1 of 1 computers') +}) + +it('promises to keep new computers turned on only when that is the standing consent', () => { + const summary = summarizeSessionSearchComputers([fleet[0]]) + expect(sessionSearchSummarySentence(summary, true)).toBe( + 'On 1 of 1 computers New computers turn on when they can.' + ) + expect(sessionSearchSummarySentence(summary, false)).not.toContain('New computers') +}) + it('orders reachable and working first, then by name inside each group', () => { const ordered = orderSessionSearchServers([ { id: 'f', name: 'probing', state: 'checking' }, diff --git a/src/renderer/src/components/settings/session-search-computer-rollup.ts b/src/renderer/src/components/settings/session-search-computer-rollup.ts index a7a9414d7df..9a9c693e3f9 100644 --- a/src/renderer/src/components/settings/session-search-computer-rollup.ts +++ b/src/renderer/src/components/settings/session-search-computer-rollup.ts @@ -1,3 +1,5 @@ +import { translate } from '@/i18n/i18n' + /** * What one computer in the pane is doing, as far as this client can tell. * @@ -12,15 +14,62 @@ export type SessionSearchComputerEntry = { state: SessionSearchComputerState } +export type SessionSearchFleetSummary = { + on: number + total: number + offline: number + needUpdate: number + /** Reachable, new enough, and still off: exactly what Turn on all would act on. */ + turnOnable: number +} + export function isTurnOnableSessionSearchState(state: SessionSearchComputerState): boolean { return state === 'off' } -/** How many computers Turn on would actually reach: reachable, new enough, and still off. */ -export function countTurnOnableSessionSearchComputers( +export function summarizeSessionSearchComputers( entries: readonly SessionSearchComputerEntry[] -): number { - return entries.filter((entry) => isTurnOnableSessionSearchState(entry.state)).length +): SessionSearchFleetSummary { + const count = (state: SessionSearchComputerState): number => + entries.filter((entry) => entry.state === state).length + return { + on: count('on'), + total: entries.length, + offline: count('offline'), + needUpdate: count('needs-update'), + turnOnable: entries.filter((entry) => isTurnOnableSessionSearchState(entry.state)).length + } +} + +/** Sentence above the list. A segment worth zero is left out rather than printed as "0". */ +export function sessionSearchSummarySentence( + summary: SessionSearchFleetSummary, + autoEnableNewComputers: boolean +): string { + const segments = [ + translate('sessionHistory.settings.summaryOn', 'On {{on}} of {{total}} computers', { + on: summary.on, + total: summary.total + }) + ] + if (summary.offline > 0) { + segments.push( + translate('sessionHistory.settings.summaryOffline', '{{offline}} offline', { + offline: summary.offline + }) + ) + } + if (summary.needUpdate > 0) { + segments.push( + translate('sessionHistory.settings.summaryNeedUpdate', '{{needUpdate}} need an update', { + needUpdate: summary.needUpdate + }) + ) + } + const sentence = segments.join(' · ') + return autoEnableNewComputers + ? `${sentence} ${translate('sessionHistory.settings.summaryAutoEnable', 'New computers turn on when they can.')}` + : sentence } // Reachable and working first, then what the user could act on, then what they cannot. diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index fd51caec022..b115ca1a2d5 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -17979,13 +17979,16 @@ "clearedAndTurnedOff": "Search turned off and search data cleared.", "thisComputer": "This computer", "remoteServers": "Orca remote servers", - "turnOnAll": "Turn on for all {{count}}", + "summaryOn": "On {{on}} of {{total}} computers", + "summaryOffline": "{{offline}} offline", + "summaryNeedUpdate": "{{needUpdate}} need an update", + "summaryAutoEnable": "New computers turn on when they can.", + "turnOnAll": "Turn on all", "showMore": "Show {{count}} more", "showFewer": "Show fewer", "openInSidebar": "Open in the sidebar", "openInSidebarCopy": "Type what you remember, or ask an agent: “find the session where we fixed the login timeout.”", - "open": "Open", - "autoEnableArmed": "New computers turn on automatically." + "open": "Open" } }, "aiVault": {