Browse Source

fix(ui) Fix links in settings when customer-domains is active (#42269)

* Fix up breadcrumb links
* Fix up team settings tab links
* Fix up server side URL inflections as well.
Mark Story 2 years ago
parent
commit
6586845237

+ 5 - 2
src/sentry/api/utils.py

@@ -244,10 +244,13 @@ _path_patterns: List[Tuple[re.Pattern[str], str]] = [
     # /organizations/slug/section, but not /organizations/new
     (re.compile(r"\/?organizations\/(?!new)[^\/]+\/(.*)"), r"/\1"),
     # For /settings/:orgId/ -> /settings/organization/
-    (re.compile(r"\/settings\/(?!account)[^\/]+\/?$"), "/settings/organization/"),
+    (
+        re.compile(r"\/settings\/(?!account)(?!projects)(?!teams)[^\/]+\/?$"),
+        "/settings/organization/",
+    ),
     # Move /settings/:orgId/:section -> /settings/:section
     # but not /settings/organization or /settings/projects which is a new URL
-    (re.compile(r"\/?settings\/(?!projects)(?!account)[^\/]+\/(.*)"), r"/settings/\1"),
+    (re.compile(r"\/?settings\/(?!account)(?!projects)(?!teams)[^\/]+\/(.*)"), r"/settings/\1"),
     (re.compile(r"\/?join-request\/[^\/]+\/?.*"), r"/join-request/"),
     (re.compile(r"\/?onboarding\/[^\/]+\/(.*)"), r"/onboarding/\1"),
     (re.compile(r"\/?[^\/]+\/([^\/]+)\/getting-started\/(.*)"), r"/getting-started/\1/\2"),

+ 2 - 0
static/app/utils/withDomainRequired.spec.tsx

@@ -59,6 +59,8 @@ describe('normalizeUrl', function () {
         '/settings/projects/python/filters/discarded/',
         '/settings/projects/python/filters/discarded/',
       ],
+      // Team settings links in breadcrumbs can be pre-normalized from breadcrumbs
+      ['/settings/teams/peeps/', '/settings/teams/peeps/'],
     ];
     for (const [input, expected] of cases) {
       result = normalizeUrl(input);

+ 2 - 2
static/app/utils/withDomainRequired.tsx

@@ -8,10 +8,10 @@ const NORMALIZE_PATTERNS: Array<[pattern: RegExp, replacement: string]> = [
   // /organizations/slug/section, but not /organizations/new
   [/\/?organizations\/(?!new)[^\/]+\/(.*)/, '/$1'],
   // For /settings/:orgId/ -> /settings/organization/
-  [/\/settings\/(?!account)[^\/]+\/?$/, '/settings/organization/'],
+  [/\/settings\/(?!account)(?!projects)(?!teams)[^\/]+\/?$/, '/settings/organization/'],
   // Move /settings/:orgId/:section -> /settings/:section
   // but not /settings/organization or /settings/projects which is a new URL
-  [/\/?settings\/(?!projects)(?!account)[^\/]+\/(.*)/, '/settings/$1'],
+  [/\/?settings\/(?!account)(?!projects)(?!teams)[^\/]+\/(.*)/, '/settings/$1'],
   [/\/?join-request\/[^\/]+\/?.*/, '/join-request/'],
   [/\/?onboarding\/[^\/]+\/(.*)/, '/onboarding/$1'],
   [/\/?[^\/]+\/([^\/]+)\/getting-started\/(.*)/, '/getting-started/$1/$2'],

+ 2 - 6
static/app/views/settings/components/settingsBreadcrumb/organizationCrumb.tsx

@@ -64,16 +64,12 @@ const OrganizationCrumb = ({
   }
 
   const hasMenu = organizations.length > 1;
+  const orgSettings = `/settings/${organization.slug}/`;
 
   return (
     <BreadcrumbDropdown
       name={
-        <CrumbLink
-          to={recreateRoute(route, {
-            routes,
-            params: {...params, orgId: organization.slug},
-          })}
-        >
+        <CrumbLink to={orgSettings}>
           <BadgeWrapper>
             <IdBadge avatarSize={18} organization={organization} />
           </BadgeWrapper>

+ 5 - 7
static/app/views/settings/components/settingsBreadcrumb/teamCrumb.tsx

@@ -4,6 +4,7 @@ import debounce from 'lodash/debounce';
 import IdBadge from 'sentry/components/idBadge';
 import {DEFAULT_DEBOUNCE_DURATION} from 'sentry/constants';
 import recreateRoute from 'sentry/utils/recreateRoute';
+import {useParams} from 'sentry/utils/useParams';
 import useTeams from 'sentry/utils/useTeams';
 import BreadcrumbDropdown from 'sentry/views/settings/components/settingsBreadcrumb/breadcrumbDropdown';
 import MenuItem from 'sentry/views/settings/components/settingsBreadcrumb/menuItem';
@@ -12,8 +13,9 @@ import {CrumbLink} from '.';
 
 type Props = RouteComponentProps<{teamId: string}, {}>;
 
-const TeamCrumb = ({params, routes, route, ...props}: Props) => {
+const TeamCrumb = ({routes, route, ...props}: Props) => {
   const {teams, onSearch, fetching} = useTeams();
+  const params = useParams();
 
   const team = teams.find(({slug}) => slug === params.teamId);
   const hasMenu = teams.length > 1;
@@ -26,16 +28,12 @@ const TeamCrumb = ({params, routes, route, ...props}: Props) => {
   if (!team) {
     return null;
   }
+  const teamUrl = `/settings/${params.orgId}/teams/${team.slug}/`;
 
   return (
     <BreadcrumbDropdown
       name={
-        <CrumbLink
-          to={recreateRoute(route, {
-            routes,
-            params: {...params, teamId: team.slug},
-          })}
-        >
+        <CrumbLink to={teamUrl}>
           <IdBadge avatarSize={18} team={team} />
         </CrumbLink>
       }

+ 18 - 5
static/app/views/settings/organizationTeams/teamDetails.spec.jsx

@@ -1,3 +1,4 @@
+import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import {Client} from 'sentry/api';
@@ -10,7 +11,6 @@ describe('TeamMembers', () => {
   const organization = TestStubs.Organization();
   const team = TestStubs.Team({hasAccess: false});
   const teamHasAccess = TestStubs.Team({id: '1337', slug: 'django', hasAccess: true});
-  const context = TestStubs.routerContext();
 
   beforeEach(() => {
     TeamStore.init();
@@ -27,13 +27,20 @@ describe('TeamMembers', () => {
   });
 
   it('can request membership', () => {
+    const {routerContext} = initializeOrg({
+      organization,
+      router: {
+        params: {orgId: organization.slug, teamId: team.slug},
+      },
+    });
+
     render(
-      <TeamDetails params={{orgId: organization.slug, teamId: team.slug}}>
+      <TeamDetails params={routerContext.context.router.params}>
         <div data-test-id="test" />
       </TeamDetails>,
       {
         organization,
-        context,
+        context: routerContext,
       }
     );
 
@@ -44,13 +51,19 @@ describe('TeamMembers', () => {
   });
 
   it('displays children', () => {
+    const {routerContext} = initializeOrg({
+      organization,
+      router: {
+        params: {orgId: organization.slug, teamId: teamHasAccess.slug},
+      },
+    });
     render(
-      <TeamDetails params={{orgId: organization.slug, teamId: teamHasAccess.slug}}>
+      <TeamDetails params={routerContext.context.router.params}>
         <div data-test-id="test" />
       </TeamDetails>,
       {
         organization,
-        context,
+        context: routerContext,
       }
     );
 

+ 6 - 11
static/app/views/settings/organizationTeams/teamDetails.tsx

@@ -12,8 +12,8 @@ import LoadingIndicator from 'sentry/components/loadingIndicator';
 import NavTabs from 'sentry/components/navTabs';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t, tct} from 'sentry/locale';
-import recreateRoute from 'sentry/utils/recreateRoute';
 import useApi from 'sentry/utils/useApi';
+import {useParams} from 'sentry/utils/useParams';
 import useTeams from 'sentry/utils/useTeams';
 
 type Props = {
@@ -22,9 +22,10 @@ type Props = {
 
 function TeamDetails({children, ...props}: Props) {
   const api = useApi();
+  const params = useParams();
   const [requesting, setRequesting] = useState(false);
-  const {teams, initiallyLoaded} = useTeams({slugs: [props.params.teamId]});
-  const team = teams.find(({slug}) => slug === props.params.teamId);
+  const {teams, initiallyLoaded} = useTeams({slugs: [params.teamId]});
+  const team = teams.find(({slug}) => slug === params.teamId);
 
   function handleRequestAccess(teamSlug: string) {
     setRequesting(true);
@@ -32,7 +33,7 @@ function TeamDetails({children, ...props}: Props) {
     joinTeam(
       api,
       {
-        orgId: props.params.orgId,
+        orgId: params.orgId,
         teamId: teamSlug,
       },
       {
@@ -56,13 +57,7 @@ function TeamDetails({children, ...props}: Props) {
     );
   }
 
-  // `/organizations/${orgId}/teams/${teamId}`;
-  const routePrefix = recreateRoute('', {
-    routes: props.routes,
-    params: props.params,
-    stepBack: -1,
-  });
-
+  const routePrefix = `/settings/${params.orgId}/teams/${params.teamId}/`;
   const navigationTabs = [
     <ListLink key={0} to={`${routePrefix}members/`}>
       {t('Members')}

+ 1 - 0
tests/sentry/api/test_utils.py

@@ -117,6 +117,7 @@ def test_customer_domain_path():
             "/settings/projects/python/filters/discarded/",
             "/settings/projects/python/filters/discarded/",
         ],
+        ["/settings/teams/peeps/", "/settings/teams/peeps/"],
     ]
     for input_path, expected in scenarios:
         assert expected == customer_domain_path(input_path)