Merge pull request 'fix(preferences): an action that answers with an address opens it; a section says how a connection ended' (#214) from fix/process-link-and-preference-redirect into main
maven / build (push) Successful in 10m17s
build / maven (push) Successful in 10m17s

This commit was merged in pull request #214.
This commit is contained in:
james.maes committed 2026-11-11 12:38:10 +00:00
commit 787a227602
10 files changed
+257 -5

No files matched your search

+4
View File
@@ -16,6 +16,10 @@
## Product fixes
- **A preference action that connects another service takes you there.** A provider that starts an OAuth connection (Connect Slack, for example) answers with the address of the service's consent page in the action result's `message`, since the UI API has no redirect field. That address was shown as the text of a success toast: long, not clickable and gone in seconds. A message that is nothing but one absolute `https:` address is now where the browser goes, in the same tab, and is never shown. A message that only mentions an address is still a message. Plain `http:`, addresses with credentials and every other scheme are not followed.
- **Preferences says how a connection ended.** A flow that left the dashboard returns to `/app/preferences?section=<name>&result=<code>` when it succeeded or `&error=<code>` when it did not. The section now shows the outcome once, above its form, as the section's label and the code as words (`Slack: connected.`, `Slack: authorization denied.`), as a status or an alert, and takes it off the address so a reload does not repeat it. Only a fixed lowercase code is read from the address, never a sentence.
- **Links in process HTML look like links.** An `HTML` process component rendered links in the text color with no underline, so a link the backend put on a screen (a bulk load template, a "Continue to Slack" link) could not be told from the words around it. They now use the link color and an underline; a link's own inline style still wins.
- **The Ask button's activity dot sits on its icon.** While a reply was being written, the dot was placed in the bottom-right corner of the button's hit area, which has no visible edge and, from 1440 px, extends past the "Ask" label, so the dot floated under and to the right of the label. The dot and the unread badge are now positioned against the sparkles icon, at every width and with the panel open or closed. The bell and Jobs badges use the same anchor and keep their place.
- **Logout no longer races an in-flight request.** Logout ends the API client's session generation first: every request still in flight (axios and the realtime and assistant streams) is aborted, and any response of the old generation that still arrives is discarded, so none can refresh the `sessionId` cookie the logout expires or change client auth state (SEC-037, SEC-048).
@@ -32,8 +32,15 @@ vi.mock('@/lib/hooks/use-container-width', () => ({
useContainerWidth: () => [undefined, layout.width],
}))
vi.mock('sonner', () => ({ toast: { success: vi.fn() } }))
const navigation = vi.hoisted(() => ({ leaveTo: vi.fn() }))
vi.mock('@/lib/utils/external-navigation', async (original) => ({
...(await original<typeof import('@/lib/utils/external-navigation')>()),
leaveTo: navigation.leaveTo,
}))
beforeEach(() => {
layout.width = 500
navigation.leaveTo.mockClear()
vi.mocked(toast.success).mockClear()
})
const tokens = preferencesFixture.sections.find((section) => section.name === 'access-tokens')!
function renderForm(section: PreferenceSection = structuredClone(tokens)) {
@@ -387,3 +394,36 @@ it('shows each returned secret and restores focus to the returned row after ackn
expect(screen.queryByDisplayValue('recovery-test-only')).not.toBeInTheDocument()
persist.mockRestore()
})
it('sends the browser to an address-only result and never shows the address', async () => {
const authorize =
'https://slack.com/oauth/v2/authorize?client_id=1.2&scope=&user_scope=chat%3Awrite&redirect_uri=https%3A%2F%2Fapp.example.test%2Fqqq%2Fhooks%2Fslack%2Foauth%2Fcallback&state=abc.def'
const section = structuredClone(tokens)
section.actions!.push({ name: 'connect', label: 'Connect service', scope: 'SECTION' })
server.use(
http.post('/qqq/v1/preferences/access-tokens/actions/connect', () =>
HttpResponse.json({ section, message: authorize })
)
)
renderForm(section)
await userEvent.setup().click(screen.getByRole('button', { name: 'Connect service' }))
await waitFor(() => expect(navigation.leaveTo).toHaveBeenCalledWith(authorize))
expect(toast.success).not.toHaveBeenCalled()
expect(screen.queryByText(/slack\.com/)).not.toBeInTheDocument()
})
it('still shows a result that only mentions an address as a message', async () => {
const section = structuredClone(tokens)
section.actions!.push({ name: 'connect', label: 'Connect service', scope: 'SECTION' })
server.use(
http.post('/qqq/v1/preferences/access-tokens/actions/connect', () =>
HttpResponse.json({ section, message: 'Connected. See https://example.test/help for more.' })
)
)
renderForm(section)
await userEvent.setup().click(screen.getByRole('button', { name: 'Connect service' }))
await waitFor(() =>
expect(toast.success).toHaveBeenCalledWith('Connected. See https://example.test/help for more.')
)
expect(navigation.leaveTo).not.toHaveBeenCalled()
})
@@ -32,6 +32,7 @@ import type {
} from '@/types/preferences'
import { runPreferenceAction } from '@/lib/api/preferences'
import { queryKeys } from '@/lib/query-client'
import { continuationUrl, leaveTo } from '@/lib/utils/external-navigation'
import { BUTTON_CLASS, buttonClass } from '@/components/ui/controls'
import { MetadataIcon } from '@/components/layout/MetadataIcon'
import { PreferenceField } from './PreferenceField'
@@ -381,7 +382,11 @@ export function usePreferenceActions({
showingSecrets.current = Boolean(result.secrets?.length)
onSaved(result.section)
setSelected(null)
if (result.message) toast.success(result.message)
// An answer that is only an address (the consent page of a service being connected) is
// where the browser goes next. It is never shown: nobody can use an address in a toast.
const destination = continuationUrl(result.message)
if (destination) leaveTo(destination)
else if (result.message) toast.success(result.message)
setSecrets(result.secrets ?? [])
} catch (error) {
if (mounted.current) setErrors(preferenceMessages(error))
@@ -22,7 +22,7 @@ import { beforeEach, expect, it, vi } from 'vitest'
import { axe } from 'jest-axe'
import { server } from '@/mocks/node'
import { ThemeProvider } from '@/lib/theme/theme-provider'
import { PreferencesPage } from './PreferencesPage'
import { PreferencesPage, sectionOutcome } from './PreferencesPage'
const state = vi.hoisted(() => ({
search: '',
@@ -107,3 +107,42 @@ it('offers retry on errors and keeps local display settings accessible', async (
await userEvent.setup().click(screen.getByRole('button', { name: 'Retry' }))
await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument())
})
it('says once that a flow that returned to a section succeeded, and takes the outcome off the address', async () => {
state.search = 'section=regional&result=connected'
renderPage()
await screen.findByRole('heading', { name: 'Regional settings' })
const outcome = screen.getByText('Regional settings: connected.')
expect(outcome).toHaveAttribute('role', 'status')
expect(outcome).toHaveAttribute('data-qqq-id', 'preferences-section-outcome')
expect(state.replace).toHaveBeenCalledWith('/app/preferences?section=regional', { scroll: false })
await userEvent.setup().click(screen.getByRole('button', { name: /Notifications What reaches/ }))
expect(screen.queryByText('Regional settings: connected.')).not.toBeInTheDocument()
})
it('names the reason a flow that returned to a section failed', async () => {
state.search = 'section=regional&error=authorization_denied'
renderPage()
expect(await screen.findByRole('alert')).toHaveTextContent(
'Regional settings: authorization denied.'
)
})
it('reads only fixed outcome codes from the address, never a sentence', () => {
const read = (search: string) => sectionOutcome(new URLSearchParams(search))
expect(read('section=slack&result=connected')).toEqual({
section: 'slack',
failed: false,
words: 'connected',
})
expect(read('section=slack&error=sign_in_required&result=connected')).toEqual({
section: 'slack',
failed: true,
words: 'sign in required',
})
expect(read('section=slack')).toBeNull()
expect(read('result=connected')).toBeNull()
expect(read('section=slack&error=Call+555-0100+now')).toBeNull()
expect(read('section=slack&error=%3Cb%3Ex%3C%2Fb%3E')).toBeNull()
expect(read(`section=slack&error=${'a_'.repeat(20)}a`)).toBeNull()
})
+50 -2
View File
@@ -17,13 +17,14 @@
/** @file Preferences route content, with device display settings and server-owned sections. */
'use client'
import { useEffect } from 'react'
import { useEffect, useState } from 'react'
import { useRouter, useSearchParams } from 'next/navigation'
import { useQuery } from '@tanstack/react-query'
import { getPreferences } from '@/lib/api/preferences'
import { queryKeys } from '@/lib/query-client'
import { useQContext } from '@/lib/context/q-context'
import { useMediaQuery } from '@/lib/hooks/use-media-query'
import { cn } from '@/lib/utils/cn'
import { BUTTON_CLASS } from '@/components/ui/controls'
import { MetadataIcon } from '@/components/layout/MetadataIcon'
import { DisplayPreferencesPanel } from '@/components/layout/DisplayPreferencesPanel'
@@ -43,6 +44,30 @@ export function PreferencesSkeleton() {
)
}
/** What a flow that left the dashboard reports when it returns to a section. */
export interface SectionOutcome {
section: string
failed: boolean
/** The provider's outcome code as words: `authorization_denied` reads "authorization denied". */
words: string
}
/** Reads the outcome a flow that left the dashboard (a consent page of a connected service)
* puts on the address when it returns: `?section=<name>&result=<code>` when it succeeded,
* `&error=<code>` when it did not. The code is one of the provider's fixed lowercase names
* (`connected`, `authorization_denied`). It is shown as words after the section's own label;
* anything else on the address is ignored, so a link cannot put its own sentence on the page.
* @param params - The page's query string.
* @returns The outcome, or `null` when the address reports none.
*/
export function sectionOutcome(params: URLSearchParams): SectionOutcome | null {
const section = params.get('section')
const error = params.get('error')
const code = error ?? params.get('result')
if (!section || !code || !/^[a-z]+(?:_[a-z]+){0,7}$/.test(code)) return null
return { section, failed: error !== null, words: code.replace(/_/g, ' ') }
}
/** Every registered section and a client-only Display section, selected through the URL.
* @returns Preferences navigation and selected section.
*/
@@ -59,6 +84,13 @@ export function PreferencesPage() {
})
// A client-only key cannot collide with the backend's [a-z] section names.
const requested = params.get('section')
const [outcome, setOutcome] = useState(() => sectionOutcome(params))
const reported = params.has('result') || params.has('error')
// An outcome is reported once: off the address it goes, so a reload does not repeat it.
useEffect(() => {
if (reported && requested)
router.replace(`/app/preferences?section=${encodeURIComponent(requested)}`, { scroll: false })
}, [reported, requested, router])
const selected =
requested === 'display' && !query.data?.sections.some((section) => section.name === 'display')
? '__display'
@@ -73,13 +105,15 @@ export function PreferencesPage() {
...(query.data?.sections ?? []),
]
const section = query.data?.sections.find((section) => section.name === selected)
const navigate = (name: string) =>
const navigate = (name: string) => {
setOutcome(null)
router.replace(
name === '__display'
? '/app/preferences'
: `/app/preferences?section=${encodeURIComponent(name)}`,
{ scroll: false }
)
}
return (
<div className="mx-auto w-full max-w-7xl space-y-6" data-qqq-id="preferences-page">
<h1 className="text-2xl font-semibold text-foreground">Preferences</h1>
@@ -159,6 +193,20 @@ export function PreferencesPage() {
<PreferencesSkeleton />
) : section ? (
<>
{outcome?.section === section.name && (
<p
role={outcome.failed ? 'alert' : 'status'}
className={cn(
'rounded-md border p-3 text-sm',
outcome.failed
? 'border-destructive text-destructive'
: 'border-border bg-muted text-foreground'
)}
data-qqq-id="preferences-section-outcome"
>
{`${section.label}: ${outcome.words}.`}
</p>
)}
<PreferenceSectionForm key={section.name} section={section} />
</>
) : (
+2 -1
View File
@@ -56,9 +56,10 @@ export function HtmlComponent({ index }: HtmlComponentProps) {
const raw = values[`${step.name}.html`]
const html = useMemo(() => (typeof raw === 'string' && raw ? sanitizeProcessHtml(raw) : ''), [raw])
if (!html) return null
// links read as links: without it they inherit the text color and have no underline
return (
<div
className="prose prose-sm max-w-none text-foreground"
className="prose prose-sm max-w-none text-foreground [&_a]:text-link [&_a]:underline"
data-qqq-id={`process-html-${index}`}
dangerouslySetInnerHTML={{ __html: html }}
/>
@@ -205,6 +205,24 @@ describe('ProcessSummaryLines', () => {
})
})
describe('HTML component', () => {
it('shows a link the backend put on the last screen as a link that opens in this tab', () => {
const address = 'https://slack.com/oauth/v2/authorize?client_id=1.2&scope=&user_scope=chat%3Awrite&state=abc.def'
const step: QFrontendStepMetaData = { name: 'result', label: 'Continue to Slack', components: [{ type: 'HTML' }] }
renderStep(step, {
noMoreSteps: true,
'result.html': `<p>Slack will ask you to approve the connection.</p><p><a href="${address.replace(/&/g, '&amp;')}" style="display:inline-block;padding:8px 16px;background:#4A154B;color:#ffffff">Continue to Slack</a></p><p>This link expires in 10 minutes.</p>`,
})
const link = screen.getByRole('link', { name: 'Continue to Slack' })
expect(link).toHaveAttribute('href', address)
expect(link).not.toHaveAttribute('target')
expect(link.getAttribute('style')).toContain('background')
expect(link.closest('[data-qqq-id="process-html-0"]')).toHaveClass('[&_a]:text-link', '[&_a]:underline')
expect(screen.getByText('This link expires in 10 minutes.')).toBeInTheDocument()
expect(screen.getByRole('button', { name: 'Return' })).toBeInTheDocument()
})
})
describe('process helpers', () => {
it('sanitizes HTML but keeps CSV and text template downloads', () => {
const html = sanitizeProcessHtml('<a href="data:text/csv;base64,QQ==" download="t.csv">t</a><a href="data:text/html,<b>x</b>">h</a><img src="x" onerror="alert(1)"><script>alert(1)</script>')
+43
View File
@@ -0,0 +1,43 @@
/*
* Copyright 2026 QRun.IO, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { expect, it } from 'vitest'
import { continuationUrl } from './external-navigation'
const AUTHORIZE =
'https://slack.com/oauth/v2/authorize?client_id=1.2&scope=&user_scope=chat%3Awrite&redirect_uri=https%3A%2F%2Fapp.example.test%2Fqqq%2Fhooks%2Fslack%2Foauth%2Fcallback&state=abc.def'
it('recognizes an answer that is only an absolute https address, and returns it unchanged', () => {
expect(continuationUrl(AUTHORIZE)).toBe(AUTHORIZE)
expect(continuationUrl(` ${AUTHORIZE}\n`)).toBe(AUTHORIZE)
})
it.each([
['an ordinary message', 'Slack disconnected.'],
['a sentence that contains an address', `Open ${AUTHORIZE}`],
['two addresses', `${AUTHORIZE} ${AUTHORIZE}`],
['plain http', 'http://slack.com/oauth/v2/authorize'],
['a script address', 'javascript:alert(1)'],
['a data address', 'data:text/html,<p>x</p>'],
['a protocol-relative address', '//slack.com/oauth'],
['a local path', '/app/preferences'],
['credentials in the address', 'https://user:secret@slack.com/oauth'],
['no host', 'https://'],
['an empty answer', ''],
['no answer', undefined],
])('leaves %s as a message', (_name, text) => {
expect(continuationUrl(text)).toBeNull()
})
+49
View File
@@ -0,0 +1,49 @@
/*
* Copyright 2026 QRun.IO, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @file external-navigation — leaving the dashboard for another site, such as the consent
* page of a service the user is connecting. An address is something to go to, never text to
* show: a backend answer that is only an address is recognized here and opened.
*/
/**
* The address a backend answer names when the answer is nothing but one absolute `https:`
* address: no other words, no whitespace, no credentials in it. A sentence that merely
* contains an address is an ordinary message and is not matched.
*
* @param text - The backend's answer (an action result message, for example).
* @returns The address exactly as sent, or `null` when the answer is anything else.
*/
export function continuationUrl(text: string | null | undefined): string | null {
const address = text?.trim()
if (!address || /\s/.test(address) || !/^https:\/\//i.test(address)) return null
try {
const url = new URL(address)
return url.protocol === 'https:' && url.hostname && !url.username && !url.password ? address : null
} catch {
return null
}
}
/**
* Sends the browser to another site in this tab, so the site can return the user here.
*
* @param url - An absolute address (see {@link continuationUrl}).
*/
export function leaveTo(url: string): void {
window.location.assign(url)
}
+5
View File
@@ -91,6 +91,11 @@ export interface PreferenceSecret {
/** Only section is safe to retain in the preferences cache. */
export interface PreferenceActionResponse {
section: PreferenceSection
/**
* The outcome, shown once. The UI API has no redirect field, so a message that is nothing but
* one absolute `https:` address is where the browser goes next (an OAuth consent page) and is
* never shown; see `continuationUrl`.
*/
message?: string
secrets?: PreferenceSecret[]
focusRowKey?: string