Browse Source

feat(customer-domains) Prototype of URL swapping (#41350)

This is a quick & dirty solution that replaces organization slugs with
slugless routes. This behavior would be enabled for customers with
domains enabled and will help us get existing customers onto new paths
while we straddle customer domains + path slugs.

Refs HC-481
Mark Story 2 years ago
parent
commit
028ec1caa5

+ 18 - 0
static/app/actionCreators/navigation.spec.jsx

@@ -97,4 +97,22 @@ describe('navigation ActionCreator', () => {
     expect(navigateTo('/settings/:orgId', router)).toBe();
     expect(openModal).toHaveBeenCalled();
   });
+
+  it('normalizes URLs for customer domains', function () {
+    window.__initialData = {
+      customerDomain: {
+        subdomain: 'albertos-apples',
+        organizationUrl: 'https://albertos-apples.sentry.io',
+        sentryUrl: 'https://sentry.io',
+      },
+    };
+    navigateTo('/settings/org-slug/projects/', router);
+    expect(openModal).not.toHaveBeenCalled();
+    expect(router.push).toHaveBeenCalledWith('/settings/projects/');
+
+    router.location.query.project = '2';
+    navigateTo('/settings/org-slug/projects/:projectId/alerts/', router);
+    expect(openModal).not.toHaveBeenCalled();
+    expect(router.push).toHaveBeenCalledWith('/settings/projects/project-slug/alerts/');
+  });
 });

+ 2 - 1
static/app/actionCreators/navigation.tsx

@@ -5,6 +5,7 @@ import {openModal} from 'sentry/actionCreators/modal';
 import ContextPickerModal from 'sentry/components/contextPickerModal';
 import ProjectsStore from 'sentry/stores/projectsStore';
 import replaceRouterParams from 'sentry/utils/replaceRouterParams';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 
 // TODO(ts): figure out better typing for react-router here
 export function navigateTo(
@@ -44,6 +45,6 @@ export function navigateTo(
         project: projectById.id,
       });
     }
-    router.push(to);
+    router.push(normalizeUrl(to));
   }
 }

+ 4 - 0
static/app/components/links/link.tsx

@@ -5,6 +5,8 @@ import styled from '@emotion/styled';
 import * as Sentry from '@sentry/react';
 import {Location, LocationDescriptor} from 'history';
 
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
+
 import {linkStyles} from './styles';
 
 export interface LinkProps
@@ -56,6 +58,8 @@ function BaseLink({
     }
   }, [location]);
 
+  to = normalizeUrl(to, location);
+
   if (!disabled && location) {
     return <RouterLink to={to} ref={forwardedRef as any} {...props} />;
   }

+ 4 - 2
static/app/components/links/listLink.tsx

@@ -5,6 +5,8 @@ import classNames from 'classnames';
 import {LocationDescriptor} from 'history';
 import * as qs from 'query-string';
 
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
+
 type LinkProps = Omit<React.ComponentProps<typeof RouterLink>, 'to'>;
 
 type Props = WithRouterProps &
@@ -39,8 +41,8 @@ function ListLink({
   ...props
 }: Props) {
   const queryData = query ? qs.parse(query) : undefined;
-  const target: LocationDescriptor =
-    typeof to === 'string' ? {pathname: to, query: queryData} : to;
+  const targetLocation = typeof to === 'string' ? {pathname: to, query: queryData} : to;
+  const target = normalizeUrl(targetLocation);
 
   const active = isActive?.(target, index) ?? router.isActive(target, index);
 

+ 99 - 1
static/app/utils/withDomainRequired.spec.tsx

@@ -1,9 +1,107 @@
 import {RouteComponentProps} from 'react-router';
+import {Location, LocationDescriptor, LocationDescriptorObject} from 'history';
 
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import withDomainRequired from 'sentry/utils/withDomainRequired';
+import withDomainRequired, {normalizeUrl} from 'sentry/utils/withDomainRequired';
+
+describe('normalizeUrl', function () {
+  let result;
+
+  beforeEach(function () {
+    window.__initialData = {
+      customerDomain: {
+        subdomain: 'albertos-apples',
+        organizationUrl: 'https://albertos-apples.sentry.io',
+        sentryUrl: 'https://sentry.io',
+      },
+    } as any;
+  });
+
+  it('replaces paths in strings', function () {
+    const location = TestStubs.location();
+    const cases = [
+      // input, expected
+      ['/settings/organization', '/settings/organization'],
+      ['/settings/sentry/members/', '/settings/members/'],
+      ['/settings/sentry/members/3/', '/settings/members/3/'],
+      ['/settings/sentry/teams/peeps/', '/settings/teams/peeps/'],
+      ['/settings/account/security/', '/settings/account/security/'],
+      ['/settings/account/details/', '/settings/account/details/'],
+      ['/organizations/new', '/organizations/new'],
+      ['/organizations/new/', '/organizations/new/'],
+      ['/join-request/acme', '/join-request/'],
+      ['/join-request/acme/', '/join-request/'],
+      ['/onboarding/acme/', '/onboarding/'],
+      ['/onboarding/acme/project/', '/onboarding/project/'],
+      ['/organizations/albertos-apples/issues/', '/issues/'],
+      ['/organizations/albertos-apples/issues/?_q=all#hash', '/issues/?_q=all#hash'],
+      ['/acme/project-slug/getting-started/', '/getting-started/project-slug/'],
+      [
+        '/acme/project-slug/getting-started/python',
+        '/getting-started/project-slug/python',
+      ],
+      ['/settings/projects/python/filters/', '/settings/projects/python/filters/'],
+      [
+        '/settings/projects/python/filters/discarded/',
+        '/settings/projects/python/filters/discarded/',
+      ],
+    ];
+    for (const scenario of cases) {
+      result = normalizeUrl(scenario[0], location);
+      expect(result).toEqual(scenario[1]);
+    }
+  });
+
+  it('replaces pathname in objects', function () {
+    const location = TestStubs.location();
+    result = normalizeUrl({pathname: '/settings/organization'}, location);
+    // @ts-ignore
+    expect(result.pathname).toEqual('/settings/organization');
+
+    result = normalizeUrl({pathname: '/settings/sentry/members'}, location);
+    // @ts-ignore
+    expect(result.pathname).toEqual('/settings/members');
+
+    result = normalizeUrl({pathname: '/organizations/albertos-apples/issues'}, location);
+    // @ts-ignore
+    expect(result.pathname).toEqual('/issues');
+
+    result = normalizeUrl(
+      {
+        pathname: '/organizations/albertos-apples/issues',
+        query: {q: 'all'},
+      },
+      location
+    );
+    // @ts-ignore
+    expect(result.pathname).toEqual('/issues');
+  });
+
+  it('replaces pathname in function callback', function () {
+    const location = TestStubs.location();
+    function objectCallback(_loc: Location): LocationDescriptorObject {
+      return {pathname: '/settings/organization'};
+    }
+    result = normalizeUrl(objectCallback, location);
+    // @ts-ignore
+    expect(result.pathname).toEqual('/settings/organization');
+
+    function stringCallback(_loc: Location): LocationDescriptor {
+      return '/organizations/a-long-slug/discover/';
+    }
+    result = normalizeUrl(stringCallback, location);
+    expect(result).toEqual('/discover/');
+  });
+
+  it('errors on functions without location', function () {
+    function objectCallback(_loc: Location): LocationDescriptorObject {
+      return {pathname: '/settings/organization'};
+    }
+    expect(() => normalizeUrl(objectCallback)).toThrow();
+  });
+});
 
 describe('withDomainRequired', function () {
   type Props = RouteComponentProps<{orgId: string}, {}>;

+ 57 - 0
static/app/utils/withDomainRequired.tsx

@@ -1,7 +1,64 @@
 import {RouteComponent, RouteComponentProps} from 'react-router';
+import {Location, LocationDescriptor} from 'history';
 import trimEnd from 'lodash/trimEnd';
 import trimStart from 'lodash/trimStart';
 
+const NORMALIZE_PATTERNS: Array<[pattern: RegExp, replacement: string]> = [
+  // /organizations/slug/section, but not /organizations/new
+  [/\/?organizations\/(?!new)[^\/]+\/(.*)/, '/$1'],
+  // /settings/slug/section but not /settings/organization
+  // and not /settings/projects which is a new URL
+  [/\/?settings\/(?!projects)(?!account)[^\/]+\/(.*)/, '/settings/$1'],
+  [/\/?join-request\/[^\/]+\/?.*/, '/join-request/'],
+  [/\/?onboarding\/[^\/]+\/(.*)/, '/onboarding/$1'],
+  [/\/?[^\/]+\/([^\/]+)\/getting-started\/(.*)/, '/getting-started/$1/$2'],
+];
+
+type LocationTarget = ((location: Location) => LocationDescriptor) | LocationDescriptor;
+
+/**
+ * Normalize a URL for customer domains based on the current route state
+ */
+export function normalizeUrl(path: string): string;
+export function normalizeUrl(path: LocationDescriptor): LocationDescriptor;
+export function normalizeUrl(path: LocationTarget, location?: Location): LocationTarget;
+export function normalizeUrl(path: LocationTarget, location?: Location): LocationTarget {
+  if (!window.__initialData?.customerDomain) {
+    return path;
+  }
+
+  let resolved: LocationDescriptor;
+  if (typeof path === 'function') {
+    if (!location) {
+      throw new Error('Cannot resolve function URL without a location');
+    }
+    resolved = path(location);
+  } else {
+    resolved = path;
+  }
+
+  if (typeof resolved === 'string') {
+    for (const patternData of NORMALIZE_PATTERNS) {
+      resolved = resolved.replace(patternData[0], patternData[1]);
+      if (resolved !== path) {
+        return resolved;
+      }
+    }
+    return resolved;
+  }
+  if (!resolved.pathname) {
+    return resolved;
+  }
+  for (const patternData of NORMALIZE_PATTERNS) {
+    resolved.pathname = resolved.pathname.replace(patternData[0], patternData[1]);
+    if (resolved !== path) {
+      return resolved;
+    }
+  }
+
+  return resolved;
+}
+
 /**
  * withDomainRequired is a higher-order component (HOC) meant to be used with <Route /> components within
  * static/app/routes.tsx whose route paths do not contain the :orgId parameter.

+ 5 - 4
static/app/views/issueList/header.tsx

@@ -17,6 +17,7 @@ import space from 'sentry/styles/space';
 import {Organization, SavedSearch} from 'sentry/types';
 import {trackAnalyticsEvent} from 'sentry/utils/analytics';
 import useProjects from 'sentry/utils/useProjects';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import IssueListSetAsDefault from 'sentry/views/issueList/issueListSetAsDefault';
 
 import SavedSearchTab from './savedSearchTab';
@@ -163,7 +164,7 @@ function IssueListHeader({
             {[
               ...visibleTabs.map(
                 ([tabQuery, {name: queryName, tooltipTitle, tooltipHoverable}]) => {
-                  const to = {
+                  const to = normalizeUrl({
                     query: {
                       ...queryParms,
                       query: tabQuery,
@@ -173,7 +174,7 @@ function IssueListHeader({
                           : sortParam,
                     },
                     pathname: `/organizations/${organization.slug}/issues/`,
-                  };
+                  });
 
                   return (
                     <Item key={tabQuery} to={to} textValue={queryName}>
@@ -208,7 +209,7 @@ function IssueListHeader({
         <Layout.HeaderNavTabs underlined>
           {visibleTabs.map(
             ([tabQuery, {name: queryName, tooltipTitle, tooltipHoverable}]) => {
-              const to = {
+              const to = normalizeUrl({
                 query: {
                   ...queryParms,
                   query: tabQuery,
@@ -216,7 +217,7 @@ function IssueListHeader({
                     tabQuery === Query.FOR_REVIEW ? IssueSortOptions.INBOX : sortParam,
                 },
                 pathname: `/organizations/${organization.slug}/issues/`,
-              };
+              });
 
               return (
                 <li key={tabQuery} className={query === tabQuery ? 'active' : ''}>

+ 2 - 1
static/app/views/issueList/overview.tsx

@@ -53,6 +53,7 @@ import withRouteAnalytics, {
   WithRouteAnalyticsProps,
 } from 'sentry/utils/routeAnalytics/withRouteAnalytics';
 import withApi from 'sentry/utils/withApi';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import withIssueTags from 'sentry/utils/withIssueTags';
 import withOrganization from 'sentry/utils/withOrganization';
 import withPageFilters from 'sentry/utils/withPageFilters';
@@ -927,7 +928,7 @@ class IssueListOverview extends Component<Props, State> {
       !isEqual(query, this.props.location.query)
     ) {
       browserHistory.push({
-        pathname: path,
+        pathname: normalizeUrl(path),
         query,
       });
       this.setState({issuesLoading: true});

+ 2 - 1
static/app/views/performance/transactionSummary/pageLayout.tsx

@@ -19,6 +19,7 @@ import EventView from 'sentry/utils/discover/eventView';
 import {useMetricsCardinalityContext} from 'sentry/utils/performance/contexts/metricsCardinality';
 import {PerformanceEventViewProvider} from 'sentry/utils/performance/contexts/performanceEventViewContext';
 import {decodeScalar} from 'sentry/utils/queryString';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 
 import {getSelectedProjectPlatforms, getTransactionName} from '../utils';
 
@@ -165,7 +166,7 @@ function PageLayout(props: Props) {
         });
       }
 
-      browserHistory.push(getNewRoute(newTab));
+      browserHistory.push(normalizeUrl(getNewRoute(newTab)));
     },
     [getNewRoute, tab, organization, location, projects]
   );

+ 48 - 0
static/app/views/settings/components/settingsBreadcrumb/organizationCrumb.spec.jsx

@@ -8,6 +8,7 @@ import OrganizationCrumb from 'sentry/views/settings/components/settingsBreadcru
 jest.unmock('sentry/utils/recreateRoute');
 
 describe('OrganizationCrumb', function () {
+  let initialData;
   const {organization, project, routerContext} = initializeOrg();
   const organizations = [
     organization,
@@ -34,7 +35,12 @@ describe('OrganizationCrumb', function () {
     );
 
   beforeEach(function () {
+    initialData = window.__initialData;
     browserHistory.push.mockReset();
+    window.location.assign.mockReset();
+  });
+  afterEach(function () {
+    window.__initalData = initialData;
   });
 
   it('switches organizations on settings index', function () {
@@ -120,4 +126,46 @@ describe('OrganizationCrumb', function () {
 
     expect(browserHistory.push).toHaveBeenCalledWith('/settings/org-slug2/');
   });
+
+  it('switches organizations for child route with customer domains', function () {
+    window.__initialData = {
+      customerDomain: {
+        subdomain: 'albertos-apples',
+        organizationUrl: 'https://albertos-apples.sentry.io',
+        sentryUrl: 'https://sentry.io',
+      },
+    };
+
+    const routes = [
+      {path: '/', childRoutes: []},
+      {childRoutes: []},
+      {path: '/foo/', childRoutes: []},
+      {childRoutes: []},
+      {path: ':bar', childRoutes: []},
+      {path: '/settings/', name: 'Settings'},
+      {name: 'Organizations', path: ':orgId/', childRoutes: []},
+      {childRoutes: []},
+      {path: 'api-keys/', name: 'API Key'},
+    ];
+    const route = routes[6];
+    const orgs = [
+      organization,
+      TestStubs.Organization({
+        id: '234',
+        slug: 'acme',
+        features: ['customer-domains'],
+        links: {
+          organizationUrl: 'https://acme.sentry.io',
+        },
+      }),
+    ];
+
+    renderComponent({routes, route, organizations: orgs});
+    switchOrganization();
+
+    // The double slug doesn't actually show up as we have more routing context present.
+    expect(window.location.assign).toHaveBeenCalledWith(
+      'https://acme.sentry.io/settings/acme/api-keys/'
+    );
+  });
 });

Some files were not shown because too many files changed in this diff