diff --git a/static/app/views/onboarding/components/scmProviderOrder.ts b/static/app/views/onboarding/components/scmProviderOrder.ts new file mode 100644 index 000000000000..cd4e4bce89c4 --- /dev/null +++ b/static/app/views/onboarding/components/scmProviderOrder.ts @@ -0,0 +1,40 @@ +import type {IntegrationProvider} from 'sentry/types/integrations'; + +/** + * Provider keys shown first, in this display order. In ScmProviderPills these + * render as top-level pill buttons; everything else follows in its original + * order (grouped into the "More" dropdown there). + */ +const PRIMARY_PROVIDER_KEYS: readonly string[] = ['github', 'gitlab', 'bitbucket']; + +/** Sort rank for a provider key: primaries by their index, everything else after. */ +function providerOrderRank(key: string): number { + const index = PRIMARY_PROVIDER_KEYS.indexOf(key); + return index === -1 ? PRIMARY_PROVIDER_KEYS.length : index; +} + +/** + * Stable-sorts items into the canonical SCM provider display order -- primary + * providers first (in PRIMARY_PROVIDER_KEYS order), then the rest in their + * original order -- keyed by each item's provider key. Used for both provider + * lists and integration lists so they share one ordering. + */ +export function sortByScmProviderOrder(items: T[], getKey: (item: T) => string): T[] { + return [...items].sort( + (a, b) => providerOrderRank(getKey(a)) - providerOrderRank(getKey(b)) + ); +} + +/** + * Splits SCM providers into the primary set (in PRIMARY_PROVIDER_KEYS order) + * and the rest (in their original order), for ScmProviderPills' pills vs. + * "More" dropdown grouping. Concatenating the two yields the same order as + * {@link sortByScmProviderOrder}. + */ +export function partitionScmProviders(providers: IntegrationProvider[]) { + const primaryProviders = PRIMARY_PROVIDER_KEYS.map(key => + providers.find(p => p.key === key) + ).filter((p): p is IntegrationProvider => p !== undefined); + const moreProviders = providers.filter(p => !PRIMARY_PROVIDER_KEYS.includes(p.key)); + return {primaryProviders, moreProviders}; +} diff --git a/static/app/views/onboarding/components/scmProviderPills.tsx b/static/app/views/onboarding/components/scmProviderPills.tsx index b6f5e3630b3b..5f69eb31f70e 100644 --- a/static/app/views/onboarding/components/scmProviderPills.tsx +++ b/static/app/views/onboarding/components/scmProviderPills.tsx @@ -9,11 +9,7 @@ import {useOrganization} from 'sentry/utils/useOrganization'; import {IntegrationButton} from 'sentry/views/settings/organizationIntegrations/integrationButton'; import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext'; -/** - * Provider keys shown as top-level pill buttons, in display order. - * Everything else is grouped into the "More" dropdown. - */ -const PRIMARY_PROVIDER_KEYS: readonly string[] = ['github', 'gitlab', 'bitbucket']; +import {partitionScmProviders} from './scmProviderOrder'; interface ScmProviderPillsProps { onInstall: (data: Integration) => void; @@ -23,10 +19,7 @@ interface ScmProviderPillsProps { export function ScmProviderPills({providers, onInstall}: ScmProviderPillsProps) { const organization = useOrganization(); const {startFlow} = useAddIntegration(); - const primaryProviders = PRIMARY_PROVIDER_KEYS.map(key => - providers.find(p => p.key === key) - ).filter((p): p is IntegrationProvider => p !== undefined); - const moreProviders = providers.filter(p => !PRIMARY_PROVIDER_KEYS.includes(p.key)); + const {primaryProviders, moreProviders} = partitionScmProviders(providers); const gridItemCount = primaryProviders.length + (moreProviders.length > 0 ? 1 : 0); const columnsXs = `repeat(${Math.min(gridItemCount, 2)}, 1fr)`; diff --git a/static/app/views/onboarding/components/useScmProviders.spec.tsx b/static/app/views/onboarding/components/useScmProviders.spec.tsx index bd75e52e6db6..2b690e609de8 100644 --- a/static/app/views/onboarding/components/useScmProviders.spec.tsx +++ b/static/app/views/onboarding/components/useScmProviders.spec.tsx @@ -37,6 +37,53 @@ describe('useScmProviders', () => { expect(result.current.scmProviders[0]!.key).toBe('github'); }); + it('orders providers with the primary providers first', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/config/integrations/`, + body: { + providers: [ + GitHubIntegrationProviderFixture({ + key: 'vsts', + slug: 'vsts', + name: 'Azure DevOps', + }), + GitHubIntegrationProviderFixture({ + key: 'bitbucket', + slug: 'bitbucket', + name: 'Bitbucket', + }), + GitHubIntegrationProviderFixture({ + key: 'github', + slug: 'github', + name: 'GitHub', + }), + GitHubIntegrationProviderFixture({ + key: 'gitlab', + slug: 'gitlab', + name: 'GitLab', + }), + ], + }, + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/`, + body: [], + }); + + const {result} = renderHookWithProviders(() => useScmProviders(), {organization}); + + await waitFor(() => expect(result.current.isPending).toBe(false)); + + // Primary providers come first in PRIMARY_PROVIDER_KEYS order, then the + // rest in their original order -- matching ScmProviderPills. + expect(result.current.scmProviders.map(p => p.key)).toEqual([ + 'github', + 'gitlab', + 'bitbucket', + 'vsts', + ]); + }); + it('returns first active integration as activeIntegrationExisting', async () => { MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/config/integrations/`, @@ -128,6 +175,113 @@ describe('useScmProviders', () => { expect(result.current.activeIntegrationExisting!.id).toBe('1'); }); + it('orders activeIntegrations by provider, prioritizing the primary choice', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/config/integrations/`, + body: {providers: []}, + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/`, + body: [ + OrganizationIntegrationsFixture({ + id: '1', + name: 'acme', + provider: { + key: 'gitlab', + slug: 'gitlab', + name: 'GitLab', + canAdd: true, + canDisable: false, + features: ['commits'], + aspects: {}, + }, + }), + OrganizationIntegrationsFixture({ + id: '2', + name: 'getsentry', + provider: { + key: 'github', + slug: 'github', + name: 'GitHub', + canAdd: true, + canDisable: false, + features: ['commits'], + aspects: {}, + }, + }), + OrganizationIntegrationsFixture({ + id: '3', + name: 'bb', + provider: { + key: 'bitbucket', + slug: 'bitbucket', + name: 'Bitbucket', + canAdd: true, + canDisable: false, + features: ['commits'], + aspects: {}, + }, + }), + ], + }); + + const {result} = renderHookWithProviders(() => useScmProviders(), {organization}); + + await waitFor(() => expect(result.current.isPending).toBe(false)); + + // Input order gitlab, github, bitbucket -> github, gitlab, bitbucket, so + // activeIntegrationExisting is the github integration. + expect(result.current.activeIntegrations.map(i => i.id)).toEqual(['2', '1', '3']); + expect(result.current.activeIntegrationExisting!.id).toBe('2'); + }); + + it('keeps two integrations of the same provider in their original order', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/config/integrations/`, + body: {providers: []}, + }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/integrations/`, + body: [ + OrganizationIntegrationsFixture({ + id: '2', + name: 'acme', + provider: { + key: 'github', + slug: 'github', + name: 'GitHub', + canAdd: true, + canDisable: false, + features: ['commits'], + aspects: {}, + }, + }), + OrganizationIntegrationsFixture({ + id: '1', + name: 'getsentry', + provider: { + key: 'github', + slug: 'github', + name: 'GitHub', + canAdd: true, + canDisable: false, + features: ['commits'], + aspects: {}, + }, + }), + ], + }); + + const {result} = renderHookWithProviders(() => useScmProviders(), {organization}); + + await waitFor(() => expect(result.current.isPending).toBe(false)); + + // Both are GitHub, so the stable sort leaves them in input order and + // activeIntegrationExisting stays the first one. + expect(result.current.activeIntegrations.map(i => i.id)).toEqual(['2', '1']); + expect(result.current.activeIntegrationExisting!.id).toBe('2'); + }); + it('excludes non-active integrations from activeIntegrationExisting', async () => { MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/config/integrations/`, diff --git a/static/app/views/onboarding/components/useScmProviders.ts b/static/app/views/onboarding/components/useScmProviders.ts index 6c1e1adbea31..c3d509dd5e01 100644 --- a/static/app/views/onboarding/components/useScmProviders.ts +++ b/static/app/views/onboarding/components/useScmProviders.ts @@ -6,6 +6,8 @@ import {apiOptions} from 'sentry/utils/api/apiOptions'; import {isScmProvider} from 'sentry/utils/integrationUtil'; import {useOrganization} from 'sentry/utils/useOrganization'; +import {sortByScmProviderOrder} from './scmProviderOrder'; + type ScmProvidersData = { // First active SCM integration, if any. Kept for callers that only support a // single integration (the onboarding connect step). @@ -42,7 +44,13 @@ export function useScmProviders(): ScmProvidersData { ); const scmProviders = useMemo( - () => (providersQuery.data?.providers ?? []).filter(isScmProvider), + () => + // Order providers the same way ScmProviderPills displays them (primary + // providers first, then the rest) so every consumer lists them alike. + sortByScmProviderOrder( + (providersQuery.data?.providers ?? []).filter(isScmProvider), + p => p.key + ), [providersQuery.data] ); @@ -59,8 +67,13 @@ export function useScmProviders(): ScmProvidersData { const activeIntegrations = useMemo( () => - (integrationsQuery.data ?? []).filter( - i => i.organizationIntegrationStatus === 'active' && i.status === 'active' + // Same provider order as scmProviders, so activeIntegrationExisting (the + // first one) prioritizes the primary providers too. + sortByScmProviderOrder( + (integrationsQuery.data ?? []).filter( + i => i.organizationIntegrationStatus === 'active' && i.status === 'active' + ), + i => i.provider.key ), [integrationsQuery.data] );