From d12e5d5c65972ee0ef7565e2a4c5af656c59a842 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 5 Aug 2026 14:41:57 +0000 Subject: [PATCH 1/3] feat: add source-control configuration removal parity --- .../SourceControlConfigForm.client.test.tsx | 171 +++++++++++- .../settings/SourceControlConfigForm.tsx | 58 +++- .../commands/source-control/index.test.ts | 256 +++++++++++++++++- .../src/trpc/commands/source-control/index.ts | 241 ++++++++++++++++- apps/web/src/trpc/routers/_app.ts | 10 +- .../bitbucket/src/__tests__/oauth.test.ts | 59 +++- packages/bitbucket/src/oauth.ts | 18 +- packages/gitea/src/__tests__/oauth.test.ts | 59 +++- packages/gitea/src/oauth.ts | 8 + packages/gitlab/src/__tests__/oauth.test.ts | 60 +++- packages/gitlab/src/oauth.ts | 10 +- 11 files changed, 899 insertions(+), 51 deletions(-) diff --git a/apps/web/src/components/settings/SourceControlConfigForm.client.test.tsx b/apps/web/src/components/settings/SourceControlConfigForm.client.test.tsx index c17e030f3..1d2ca114e 100644 --- a/apps/web/src/components/settings/SourceControlConfigForm.client.test.tsx +++ b/apps/web/src/components/settings/SourceControlConfigForm.client.test.tsx @@ -16,6 +16,7 @@ const { saveMutationOptionsRef, clearMutationOptionsRef, invalidateQueriesMock, + toastWarningMock, } = vi.hoisted(() => ({ saveMutateMock: vi.fn(), clearMutateMock: vi.fn(), @@ -27,11 +28,14 @@ const { }, clearMutationOptionsRef: { current: null as { - onSuccess?: () => Promise | void; + onSuccess?: (result: { + warnings: Array<{ message: string }>; + }) => Promise | void; onError?: (error: Error) => void; } | null, }, invalidateQueriesMock: vi.fn(async () => undefined), + toastWarningMock: vi.fn(), })); vi.mock('@tanstack/react-query', () => ({ @@ -60,7 +64,7 @@ vi.mock('@tanstack/react-query', () => ({ })); vi.mock('sonner', () => ({ - toast: { success: vi.fn(), error: vi.fn() }, + toast: { success: vi.fn(), error: vi.fn(), warning: toastWarningMock }, })); const { adoLinkedAccountRef } = vi.hoisted(() => ({ @@ -91,7 +95,7 @@ vi.mock('@/trpc/client', () => ({ mutationKey: ['saveConfig'], }), }, - clearGitHubConfig: { + clearConfig: { mutationOptions: (options: object) => ({ ...options, mutationKey: ['clearConfig'], @@ -124,21 +128,25 @@ const MASKED_VALUE = '••••••••••••••••••• function buildConfigStatus( fields: SetupSourceControlStatus['providers'][number]['fields'], + provider: SetupSourceControlStatus['preselectedProvider'] = 'github', ): SetupSourceControlStatus { + const catalogProvider = SETUP_SOURCE_CONTROL_PROVIDER_CATALOG.find( + (candidate) => candidate.provider === provider, + )!; return { - selectedProvider: 'github', - preselectedProvider: 'github', + selectedProvider: provider, + preselectedProvider: provider, runtimeConfiguredProvider: null, runtimeConfiguredProviders: [], lockReason: null, - connectedProvider: 'github', + connectedProvider: provider, setupSatisfied: true, setupSatisfiedByRuntimeEnv: false, providers: [ { - provider: 'github', - label: 'GitHub', - connectionMode: 'app', + provider, + label: catalogProvider.label, + connectionMode: catalogProvider.connectionMode, runtimeConfigSatisfied: false, savedConfigSatisfied: true, configSatisfied: true, @@ -157,6 +165,7 @@ describe('SourceControlConfigForm', () => { saveMutateMock.mockReset(); clearMutateMock.mockReset(); invalidateQueriesMock.mockClear(); + toastWarningMock.mockReset(); saveMutationOptionsRef.current = null; clearMutationOptionsRef.current = null; adoLinkedAccountRef.current = { data: undefined, isPending: false }; @@ -311,16 +320,16 @@ describe('SourceControlConfigForm', () => { }), ).toBeInTheDocument(); expect( - screen.getByText(/repositories will be disconnected/), + screen.getByText(/installations and repositories will be disconnected/), ).toBeInTheDocument(); const removeButtons = screen.getAllByRole('button', { name: 'Remove' }); fireEvent.click(removeButtons.at(-1)!); - expect(clearMutateMock).toHaveBeenCalledWith(); + expect(clearMutateMock).toHaveBeenCalledWith({ provider: 'github' }); await act(async () => { - await clearMutationOptionsRef.current?.onSuccess?.(); + await clearMutationOptionsRef.current?.onSuccess?.({ warnings: [] }); }); expect(invalidateQueriesMock).toHaveBeenCalledWith({ @@ -328,6 +337,76 @@ describe('SourceControlConfigForm', () => { }); }); + it('offers removal for saved non-GitHub configuration and displays cleanup warnings', async () => { + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Remove' })); + expect( + screen.getByRole('heading', { + name: 'Remove GitLab configuration?', + }), + ).toBeInTheDocument(); + fireEvent.click(screen.getAllByRole('button', { name: 'Remove' }).at(-1)!); + expect(clearMutateMock).toHaveBeenCalledWith({ provider: 'gitlab' }); + + await act(async () => { + await clearMutationOptionsRef.current?.onSuccess?.({ + warnings: [{ message: 'Remove the remaining hook manually.' }], + }); + }); + expect(toastWarningMock).toHaveBeenCalledWith( + 'Remove the remaining hook manually.', + ); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: ['sourceControl.configStatus'], + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: ['sourceControl.repositories'], + }); + }); + + it('keeps removal hidden for runtime-only configuration', () => { + render( + , + ); + + expect(screen.queryByRole('button', { name: 'Remove' })).toBeNull(); + }); + it('renders the Azure DevOps auth modes and advanced fields', () => { const ado = SETUP_SOURCE_CONTROL_PROVIDER_CATALOG.find( (provider) => provider.provider === 'ado', @@ -506,4 +585,72 @@ describe('SourceControlConfigForm', () => { expect(screen.getByText(/Connected as Ada Lovelace/)).toBeInTheDocument(); expect(screen.queryByText(/Not in use yet/)).not.toBeInTheDocument(); }); + + it('confirms Azure DevOps account unlinking and invalidates the linked account', async () => { + adoLinkedAccountRef.current = { + data: { + configured: true, + account: { accountId: 'ada@contoso.com', displayName: 'Ada Lovelace' }, + }, + isPending: false, + }; + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Remove' })); + expect( + screen.getByText(/delegated Azure DevOps account will be unlinked/), + ).toBeInTheDocument(); + fireEvent.click(screen.getAllByRole('button', { name: 'Remove' }).at(-1)!); + expect(clearMutateMock).toHaveBeenCalledWith({ provider: 'ado' }); + + await act(async () => { + await clearMutationOptionsRef.current?.onSuccess?.({ warnings: [] }); + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: ['linkedAccounts.ado'], + }); + }); + + it('does not promise account unlinking for Azure DevOps PAT configuration', () => { + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Remove' })); + expect( + screen.getByText( + /Existing Azure DevOps repositories will be disconnected/, + ), + ).toBeInTheDocument(); + expect( + screen.queryByText( + /configured delegated Azure DevOps account will be unlinked/, + ), + ).toBeNull(); + }); }); diff --git a/apps/web/src/components/settings/SourceControlConfigForm.tsx b/apps/web/src/components/settings/SourceControlConfigForm.tsx index 6d4b70a9a..e2c0f70cd 100644 --- a/apps/web/src/components/settings/SourceControlConfigForm.tsx +++ b/apps/web/src/components/settings/SourceControlConfigForm.tsx @@ -162,21 +162,33 @@ export function SourceControlConfigForm({ }), ); const clearConfig = useMutation( - trpc.sourceControl.clearGitHubConfig.mutationOptions({ - onSuccess: async () => { + trpc.sourceControl.clearConfig.mutationOptions({ + onSuccess: async (result) => { await queryClient.invalidateQueries({ queryKey: trpc.sourceControl.configStatus.queryKey(), }); await queryClient.invalidateQueries({ queryKey: trpc.sourceControl.repositories.queryKey(), }); - await queryClient.invalidateQueries({ - queryKey: trpc.github.installations.queryKey(), - }); + if (provider === 'github') { + await queryClient.invalidateQueries({ + queryKey: trpc.github.installations.queryKey(), + }); + } + if (provider === 'ado') { + await queryClient.invalidateQueries({ + queryKey: trpc.linkedAccounts.ado.queryKey(), + }); + } setValues({}); setEditingSavedValues({}); setRemoveDialogOpen(false); - toast.success('GitHub configuration removed.'); + toast.success( + `${providerStatus?.label ?? 'Source-control'} configuration removed.`, + ); + for (const warning of result.warnings) { + toast.warning(warning.message); + } }, onError: (error) => { toast.error(error.message); @@ -286,9 +298,23 @@ export function SourceControlConfigForm({ } return nextValue.length > 0; }); - const hasSavedValues = - provider === 'github' && - providerStatus.fields.some((field) => field.savedSatisfied); + const hasSavedValues = providerStatus.fields.some( + (field) => field.savedSatisfied, + ); + const hasPersistedAdoLinkedAccount = providerStatus.fields.some( + (field) => + field.envVarName === 'ADO_LINKED_ACCOUNT_ID' && + field.savedSatisfied && + Boolean(field.savedValue?.trim()), + ); + const providerRemovalDetail = + provider === 'github' + ? 'Existing GitHub App installations and repositories will be disconnected.' + : provider === 'ado' + ? hasPersistedAdoLinkedAccount + ? 'The configured delegated Azure DevOps account will be unlinked, and existing repositories will be disconnected.' + : 'Existing Azure DevOps repositories will be disconnected.' + : `The encrypted ${providerStatus.label} OAuth connection will be deleted, and existing repositories will be disconnected.`; return (
@@ -510,12 +536,14 @@ export function SourceControlConfigForm({ - Remove GitHub configuration? + + Remove {providerStatus.label} configuration? + - Saved GitHub credentials will be removed from the database. - Process environment variables are not affected. Existing GitHub - repositories will be disconnected so you can create or configure - another GitHub App. + Saved {providerStatus.label} configuration will be removed from + the database. Process environment variables are not affected.{' '} + {providerRemovalDetail} Roomote will attempt to remove external + hooks first. Cleanup failures will be reported after removal. @@ -530,7 +558,7 @@ export function SourceControlConfigForm({