diff --git a/backend/services/installable/toolInstallables.ts b/backend/services/installable/toolInstallables.ts index e7421b24f3..11babe5b22 100644 --- a/backend/services/installable/toolInstallables.ts +++ b/backend/services/installable/toolInstallables.ts @@ -131,7 +131,7 @@ const githubToolNames = (): string[] => toolDefinitions() export const buildGithubToolInstallable = () => ({ installableId: 'github', name: 'GitHub', - description: 'Issues and pull requests in this instance\'s own GitHub repository. Connecting your own GitHub is coming.', + description: 'Issues and pull requests in this instance\'s own GitHub repository.', version: '1.0.0', kind: 'app', source: 'builtin', diff --git a/docs/design/evidence/task-323-tools-fold-1440.png b/docs/design/evidence/task-323-tools-fold-1440.png new file mode 100644 index 0000000000..c50903ba01 Binary files /dev/null and b/docs/design/evidence/task-323-tools-fold-1440.png differ diff --git a/docs/design/evidence/task-323-tools-fold-390.png b/docs/design/evidence/task-323-tools-fold-390.png new file mode 100644 index 0000000000..308914a989 Binary files /dev/null and b/docs/design/evidence/task-323-tools-fold-390.png differ diff --git a/frontend/src/v2/__tests__/V2ConnectorTools.fold.test.tsx b/frontend/src/v2/__tests__/V2ConnectorTools.fold.test.tsx new file mode 100644 index 0000000000..78064494b4 --- /dev/null +++ b/frontend/src/v2/__tests__/V2ConnectorTools.fold.test.tsx @@ -0,0 +1,86 @@ +import React from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import V2ConnectorTools from '../components/V2ConnectorTools'; +import { AuthContext } from '../../context/AuthContext'; +import i18n, { i18nReady } from '../../i18n'; + +jest.mock('axios', () => { + const mock = { + get: jest.fn(), post: jest.fn(), patch: jest.fn(), delete: jest.fn(), + defaults: { baseURL: '', headers: { common: {} } }, + interceptors: { request: { use: jest.fn(), eject: jest.fn() }, response: { use: jest.fn(), eject: jest.fn() } }, + }; + return { __esModule: true, default: mock, ...mock }; +}); +const axios = jest.requireMock('axios').default; + +const auth = { + currentUser: { _id: 'u1', username: 'sam', email: 'sam@example.test' }, user: { _id: 'u1', username: 'sam', email: 'sam@example.test' }, token: 'user-jwt', + loading: false, error: null, isAuthenticated: true, register: jest.fn(), login: jest.fn(), signInWithToken: jest.fn(), logout: jest.fn(), updateProfile: jest.fn(), +}; +const tomorrow = new Date(Date.now() + 86_400_000).toISOString(); +const yesterday = new Date(Date.now() - 86_400_000).toISOString(); +const grant = (id: string, revokedAt: string | null) => ({ + grantId: id, installationId: 'inst-github', target: { kind: 'pod', id: 'p1' }, tools: ['github.list_issues'], + writeMode: 'read', budget: null, effectiveAudience: [], expiresAt: tomorrow, + revokedAt, revokedBy: revokedAt ? 'u1' : null, parentGrantId: null, rootGrantId: null, + createdAt: yesterday, grantedBy: 'u1', +}); +const catalog = [{ installableId: 'github', list: 'tools', label: 'GitHub', description: 'Issues', available: true, + tools: [{ name: 'github.list_issues', requiredWriteMode: 'read', irreversible: false }], connections: [] }]; + +beforeAll(async () => { await i18nReady; await i18n.changeLanguage('en'); }); + +test('two live grants stay visible and nine revoked or expired grants start in one closed fold', async () => { + const grants = [grant('live-1', null), grant('live-2', null), + ...Array.from({ length: 4 }, (_, i) => grant(`revoked-${i}`, yesterday)), + ...Array.from({ length: 5 }, (_, i) => ({ ...grant(`expired-${i}`, null), expiresAt: yesterday }))]; + axios.get.mockImplementation((url: string) => { + if (url === '/api/installables') return Promise.resolve({ data: { installables: catalog } }); + if (url === '/api/pods/p1/grants') return Promise.resolve({ data: { grants } }); + if (url === '/api/registry/pods/p1/agents') return Promise.resolve({ data: { agents: [] } }); + if (url.includes('/calls')) return Promise.resolve({ data: { calls: [], counts: { total: 0, ok: 0, refused: 0, pending_approval: 0, failed: 0 } } }); + return Promise.reject(new Error(`unmocked ${url}`)); + }); + + const { container } = render( + + ); + + const fold = await screen.findByRole('button', { name: 'inactive · 9' }); + expect(fold).toHaveAttribute('aria-expanded', 'false'); + expect(container.querySelectorAll('.v2-connector-row')).toHaveLength(2); + expect(screen.queryByText(/not enabled|not yet|coming/i)).not.toBeInTheDocument(); + + fireEvent.click(fold); + await waitFor(() => expect(container.querySelectorAll('.v2-connector-row')).toHaveLength(11)); + expect(fold).toHaveAttribute('aria-expanded', 'true'); +}); + +test('search counts only matching inactive grants, catalogue entries and connections', async () => { + const entries = [catalog[0], { ...catalog[0], installableId: 'linear', connectionType: 'hosted-mcp', entryId: 'linear', label: 'Linear', description: 'Linear issues', + tools: [{ name: 'linear.list_issues', requiredWriteMode: 'read', irreversible: false }] }]; + axios.get.mockImplementation((url: string) => { + if (url === '/api/installables') return Promise.resolve({ data: { installables: entries, hiddenToolConnections: [ + { connectionId: 'hidden-1', entryId: 'atlassian', label: 'Atlassian', status: 'inactive', revokePage: null }, + ] } }); + if (url === '/api/pods/p1/grants') return Promise.resolve({ data: { grants: [grant('live', null), grant('revoked', yesterday)] } }); + if (url === '/api/registry/pods/p1/agents') return Promise.resolve({ data: { agents: [] } }); + if (url.includes('/calls')) return Promise.resolve({ data: { calls: [], counts: { total: 0, ok: 0, refused: 0, pending_approval: 0, failed: 0 } } }); + return Promise.reject(new Error(`unmocked ${url}`)); + }); + + const { container } = render( + + ); + expect(await screen.findByRole('button', { name: 'inactive · 3' })).toBeInTheDocument(); + + fireEvent.change(screen.getByRole('searchbox', { name: 'Search tools' }), { target: { value: 'Linear' } }); + const fold = screen.getByRole('button', { name: 'inactive · 1' }); + fireEvent.click(fold); + expect(container.querySelectorAll('.v2-connector-row')).toHaveLength(1); + expect(container.querySelector('.v2-connector-row')).toHaveTextContent('Linear'); + expect(container).not.toHaveTextContent('Atlassian'); + expect(container.querySelector('.v2-connector-row__name')).toHaveTextContent('Linear'); +}); diff --git a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx index 7bde0b5efd..6d75cf05da 100644 --- a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx @@ -9,6 +9,13 @@ import V2ConnectorTools from '../components/V2ConnectorTools'; import { AuthContext } from '../../context/AuthContext'; import i18n, { i18nReady } from '../../i18n'; +// These existing cases exercise grant actions and row copy with the roster +// expanded. The collapsed interaction is covered by V2ConnectorTools.fold. +jest.mock('../components/V2InactiveFold', () => ({ + __esModule: true, + default: ({ children }) => <>{children}, +})); + jest.mock('axios', () => { const mock = { get: jest.fn(), post: jest.fn(), patch: jest.fn(), delete: jest.fn(), @@ -538,8 +545,8 @@ test('an unavailable not-yet tool row offers the same Ask affordance as unavaila return Promise.reject(new Error(`unmocked ${url}`)); }); renderTools(); - expect(await screen.findByText('not enabled on this instance · ask your operator')).toBeInTheDocument(); - const ask = screen.getByRole('link', { name: 'Ask' }); + const ask = await screen.findByRole('link', { name: 'Ask' }); + expect(screen.queryByText('not enabled on this instance · ask your operator')).not.toBeInTheDocument(); expect(ask).toHaveAttribute('href', 'https://github.com/Team-Commonly/commonly/issues/new?title=Connector%20request'); expect(ask.closest('.v2-connector-row')).toHaveClass('v2-connector-row--not-enabled'); }); @@ -676,7 +683,7 @@ test('Linear exposes only its pinned read mode in the not-yet row and Add form', expect(within(modeGroup).getByRole('button', { name: 'read' })).toHaveAttribute('aria-pressed', 'true'); }); -test('search filters by tool name and the segment hides not-yet rows; with no grants and no catalogue it renders nothing', async () => { +test('search filters by tool name; with no grants and no catalogue it renders nothing', async () => { renderTools(); await screen.findByRole('heading', { name: 'Tools' }); fireEvent.change(screen.getByRole('searchbox', { name: 'Search tools' }), { target: { value: 'comment' } }); diff --git a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx index 5f8a853f06..d568aa0136 100644 --- a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx @@ -738,6 +738,7 @@ describe('V2ConnectorsPage', () => { }], [hostedLinearCatalogEntry()]); renderPage(); + fireEvent.click(await screen.findByRole('button', { name: 'inactive · 1' })); const reviewLink = await screen.findByRole('link', { name: "Open Linear's connected apps" }); expect(reviewLink).toHaveAttribute('href', revokePage); expect(reviewLink).toHaveAttribute('target', '_blank'); @@ -766,6 +767,7 @@ describe('V2ConnectorsPage', () => { try { renderPage(); + fireEvent.click(await screen.findByRole('button', { name: '非活跃 · 1' })); const reviewLink = await screen.findByRole('link', { name: '打开 Linear 的已连接应用' }); expect(reviewLink).toHaveAttribute('href', revokePage); const notice = screen.getByRole('status'); @@ -791,6 +793,7 @@ describe('V2ConnectorsPage', () => { }], [hostedLinearCatalogEntry()]); renderPage(); + fireEvent.click(await screen.findByRole('button', { name: 'inactive · 1' })); const linearDescription = await screen.findByText('Read Linear issues and projects.'); expect(linearDescription.closest('.v2-connector-row')).toBeInTheDocument(); await waitFor(() => expect(window.location.search).toBe('')); diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index d34dda8a8d..c9bebcce6d 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -4605,3 +4605,14 @@ describe('BYO command boxes wrap at spaces', () => { expect(rule).not.toContain('word-break: break-all'); }); }); + +describe('connector inactive fold layout', () => { + const v2 = read('../v2.css'); + test('the toggle stays a full-width row with a visible keyboard focus state', () => { + const rule = ruleBody(v2, '.v2-root button.v2-connectors__inactive-toggle'); + expect(rule).toContain('width: 100%'); + expect(rule).toContain('min-height: 44px'); + expect(ruleBody(v2, '.v2-root button.v2-connectors__inactive-toggle:focus-visible')) + .toContain('box-shadow: inset 0 0 0 2px var(--v2-accent)'); + }); +}); diff --git a/frontend/src/v2/components/V2ConnectorTools.tsx b/frontend/src/v2/components/V2ConnectorTools.tsx index fbf75c98ed..135cd10693 100644 --- a/frontend/src/v2/components/V2ConnectorTools.tsx +++ b/frontend/src/v2/components/V2ConnectorTools.tsx @@ -20,6 +20,7 @@ import { V2Pod } from '../hooks/useV2Pods'; import { PlatformGlyph } from '../icons/platforms'; import { ActGlyph } from '../icons/glyphs'; import { hostedMcpErrorNotice } from './hostedMcpErrorNotice'; +import V2InactiveFold from './V2InactiveFold'; export type GrantWriteMode = 'read' | 'write' | 'write-with-confirm'; @@ -137,7 +138,6 @@ const MODE_RANK: Record = { read: 0, 'write-with-confirm // JSX, because eslint's i18next/no-literal-string (jsx-only) reads an array literal in // a JSX expression as copy — and these are ids, not words (TASK-164). const WRITE_MODES: GrantWriteMode[] = ['read', 'write-with-confirm', 'write']; -const SEGMENTS = ['all', 'granted', 'not-yet'] as const; /** The age as a unit-suffixed number ("23d", "5m", "just now"), from the timestamp and keys. */ export const shortAge = (date: string | null | undefined, now: number, t: (key: string, options?: Record) => string): string => { @@ -226,7 +226,6 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) const [joinPolicyFixPodId, setJoinPolicyFixPodId] = useState(null); const [policyFixedPodId, setPolicyFixedPodId] = useState(null); const [query, setQuery] = useState(''); - const [segment, setSegment] = useState<'all' | 'granted' | 'not-yet'>('all'); // Grants and trail rows carry relative ages; they must advance while the // page sits open and be re-read when the tab comes back (TASK-131). const now = useRelativeNow(); @@ -400,19 +399,20 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) const q = query.trim().toLowerCase(); const rows = useMemo(() => (grants || []).filter((grant) => { - if (segment === 'not-yet') return false; if (!q) return true; return toolLabel(grant).toLowerCase().includes(q) || grant.tools.some((tool) => tool.toLowerCase().includes(q)); // eslint-disable-next-line react-hooks/exhaustive-deps - }), [grants, q, segment, catalog]); - // A tool is "not yet" while no live grant on it exists anywhere the person can see. - // `now` is a dep: a grant that expires while the page sits open has to move - // back under Not yet on the same tick that drops the header count, or the - // count and the list disagree until a reload. - const notYet = useMemo(() => (segment === 'granted' ? [] : catalog.filter((entry) => ( + }), [grants, q, catalog]); + // Catalogue entries without a live grant join the inactive fold. `now` is + // a dep so expiry moves a grant into the fold without a reload. + const notYet = useMemo(() => catalog.filter((entry) => ( !entryIsGranted(entry, grants || [], now) && (!q || entry.label.toLowerCase().includes(q) || entry.tools.some((tool) => tool.name.toLowerCase().includes(q))) - ))), [catalog, grants, now, q, segment]); + )), [catalog, grants, now, q]); + const liveRows = rows.filter((grant) => !isDead(grant, now)); + const inactiveRows = rows.filter((grant) => isDead(grant, now)); + const matchedHiddenConnections = hiddenConnections.filter((connection) => !q || connection.label.toLowerCase().includes(q)); + const inactiveCount = inactiveRows.length + notYet.length + matchedHiddenConnections.length; // Grants, not entries: a person may hold two live grants on one tool. const grantedCount = (grants || []).filter((grant) => !isDead(grant, now)).length; // Entries, from the same predicate the Not yet list filters on, so the header @@ -739,7 +739,7 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) {dead && entry && isGranter(grant) ? ( // Grant again is the granter's too (Wren 67920): the mint 403s anyone but the Connection's owner. - ) : ( @@ -780,13 +780,11 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) {entry.description} - {!entry.available - ? t('tools.notEnabled', { defaultValue: 'not enabled on this instance · ask your operator' }) - : entry.connections.length === 0 + {entry.available && (entry.connections.length === 0 ? (isHosted ? t('tools.connectOwn', { defaultValue: 'connect your own {{label}} account', label: entry.label }) : t('tools.noConnection', { defaultValue: 'install the GitHub App first · an admin does this once' })) - : entryModeCopy(entry)} + : entryModeCopy(entry))} {canConnectHosted && entry.connectNotice && ( {entry.connectNotice} @@ -1084,29 +1082,19 @@ const V2ConnectorTools: React.FC = ({ pods, manualRevokeNotice = null }) aria-label={t('tools.search', { defaultValue: 'Search tools' })} onChange={(event) => setQuery(event.target.value)} /> -
- {SEGMENTS.map((key) => ( - - ))} -
{error && !draft && !selected && !githubAppSetup && (

{error}

)} {removedHiddenNotice &&

{t('tools.removedReviewProvider', { defaultValue: 'Connection removed. Review Commonly access at {{label}}.', label: removedHiddenNotice.label })} {t('tools.openProviderSettings', { defaultValue: 'Open provider settings' })}

}
- {rows.map(renderRow)} - {notYet.map(renderNotYet)} - {segment !== 'not-yet' && hiddenConnections.filter((connection) => !query || connection.label.toLowerCase().includes(q)).map(renderHiddenConnection)} - {rows.length === 0 && notYet.length === 0 && hiddenConnections.length === 0 && ( + {liveRows.map(renderRow)} + + {inactiveRows.map(renderRow)} + {notYet.map(renderNotYet)} + {matchedHiddenConnections.map(renderHiddenConnection)} + + {liveRows.length === 0 && inactiveCount === 0 && (

{t('tools.nothingMatches', { defaultValue: 'Nothing matches.' })}

)}
diff --git a/frontend/src/v2/v2.css b/frontend/src/v2/v2.css index 768501f123..7de4a06634 100644 --- a/frontend/src/v2/v2.css +++ b/frontend/src/v2/v2.css @@ -10509,7 +10509,7 @@ body.modern-ui.v2-canvas { .v2-connectors__inactive-fold { min-width: 0; } .v2-root button.v2-connectors__inactive-toggle { display: flex; align-items: center; gap: 8px; width: 100%; min-height: 44px; padding: 10px 18px; border: 0; background: var(--v2-surface); color: var(--v2-text-tertiary); font: 500 12px/16px var(--v2-font-mono); text-align: left; cursor: pointer; } .v2-root button.v2-connectors__inactive-toggle:hover { background: var(--v2-surface-hover); } -.v2-root button.v2-connectors__inactive-toggle:focus-visible { box-shadow: var(--v2-focus-ring); outline: none; } +.v2-root button.v2-connectors__inactive-toggle:focus-visible { box-shadow: inset 0 0 0 2px var(--v2-accent); outline: none; } .v2-connectors__inactive-caret { width: 7px; height: 7px; border-right: 1px solid currentColor; border-bottom: 1px solid currentColor; transform: rotate(-45deg); } .v2-root button.v2-connectors__inactive-toggle[aria-expanded="true"] .v2-connectors__inactive-caret { transform: rotate(45deg); } .v2-connectors__inactive-rows > .v2-connector-row:last-child { border-bottom: 0; }