Просмотр исходного кода

ref(feature): support single feature signature (#60683)

Refactors feature and feature disabled components to support a single
feature via string signature (I'm open to adding a feature prop instead
of overloading features if someone has strong opinions). With the
changes here we can avoid inlining the array each time + save a couple
of rerenders here and there.

Apologies to everyone getting tagged, you can ignore this. Someone from
@getsentry/app-frontend is probably best suitable to review the changes.
Jonas 1 год назад
Родитель
Сommit
d4cee1542f

+ 14 - 14
static/app/components/acl/feature.spec.tsx

@@ -62,7 +62,7 @@ describe('Feature', function () {
     });
 
     it('has no features', function () {
-      render(<Feature features={['org-baz']}>{childrenMock}</Feature>, {
+      render(<Feature features="org-baz">{childrenMock}</Feature>, {
         context: routerContext,
       });
 
@@ -78,7 +78,7 @@ describe('Feature', function () {
     it('calls render function when no features', function () {
       const noFeatureRenderer = jest.fn(() => null);
       render(
-        <Feature features={['org-baz']} renderDisabled={noFeatureRenderer}>
+        <Feature features="org-baz" renderDisabled={noFeatureRenderer}>
           {childrenMock}
         </Feature>,
         {context: routerContext}
@@ -97,7 +97,7 @@ describe('Feature', function () {
     it('can specify org from props', function () {
       const customOrg = Organization({features: ['org-bazar']});
       render(
-        <Feature organization={customOrg} features={['org-bazar']}>
+        <Feature organization={customOrg} features="org-bazar">
           {childrenMock}
         </Feature>,
         {context: routerContext}
@@ -115,7 +115,7 @@ describe('Feature', function () {
     it('can specify project from props', function () {
       const customProject = TestStubs.Project({features: ['project-baz']});
       render(
-        <Feature project={customProject} features={['project-baz']}>
+        <Feature project={customProject} features="project-baz">
           {childrenMock}
         </Feature>,
         {context: routerContext}
@@ -148,7 +148,7 @@ describe('Feature', function () {
     });
 
     it('handles features prefixed with org/project', function () {
-      render(<Feature features={['organizations:org-bar']}>{childrenMock}</Feature>, {
+      render(<Feature features="organizations:org-bar">{childrenMock}</Feature>, {
         context: routerContext,
       });
 
@@ -160,7 +160,7 @@ describe('Feature', function () {
         renderDisabled: false,
       });
 
-      render(<Feature features={['projects:bar']}>{childrenMock}</Feature>, {
+      render(<Feature features="projects:bar">{childrenMock}</Feature>, {
         context: routerContext,
       });
 
@@ -178,7 +178,7 @@ describe('Feature', function () {
         features: new Set(['organizations:create']),
       });
 
-      render(<Feature features={['organizations:create']}>{childrenMock}</Feature>, {
+      render(<Feature features="organizations:create">{childrenMock}</Feature>, {
         context: routerContext,
       });
 
@@ -195,7 +195,7 @@ describe('Feature', function () {
   describe('no children', function () {
     it('should display renderDisabled with no feature', function () {
       render(
-        <Feature features={['nope']} renderDisabled={() => <span>disabled</span>}>
+        <Feature features="nope" renderDisabled={() => <span>disabled</span>}>
           <div>The Child</div>
         </Feature>,
         {context: routerContext}
@@ -205,7 +205,7 @@ describe('Feature', function () {
 
     it('should display be empty when on', function () {
       render(
-        <Feature features={['org-bar']} renderDisabled={() => <span>disabled</span>}>
+        <Feature features="org-bar" renderDisabled={() => <span>disabled</span>}>
           <div>The Child</div>
         </Feature>,
         {context: routerContext}
@@ -217,7 +217,7 @@ describe('Feature', function () {
   describe('as React node', function () {
     it('has features', function () {
       render(
-        <Feature features={['org-bar']}>
+        <Feature features="org-bar">
           <div>The Child</div>
         </Feature>,
         {context: routerContext}
@@ -228,7 +228,7 @@ describe('Feature', function () {
 
     it('has no features', function () {
       render(
-        <Feature features={['org-baz']}>
+        <Feature features="org-baz">
           <div>The Child</div>
         </Feature>,
         {context: routerContext}
@@ -239,7 +239,7 @@ describe('Feature', function () {
 
     it('renders a default disabled component', function () {
       render(
-        <Feature features={['org-baz']} renderDisabled>
+        <Feature features="org-baz" renderDisabled>
           <div>The Child</div>
         </Feature>,
         {context: routerContext}
@@ -253,7 +253,7 @@ describe('Feature', function () {
       const noFeatureRenderer = jest.fn(() => null);
       const children = <div>The Child</div>;
       render(
-        <Feature features={['org-baz']} renderDisabled={noFeatureRenderer}>
+        <Feature features="org-baz" renderDisabled={noFeatureRenderer}>
           {children}
         </Feature>,
         {context: routerContext}
@@ -286,7 +286,7 @@ describe('Feature', function () {
     it('uses hookName if provided', function () {
       const children = <div>The Child</div>;
       render(
-        <Feature features={['org-bazar']} hookName="feature-disabled:sso-basic">
+        <Feature features="org-bazar" hookName="feature-disabled:sso-basic">
           {children}
         </Feature>,
         {context: routerContext}

+ 18 - 14
static/app/components/acl/feature.tsx

@@ -10,6 +10,8 @@ import withProject from 'sentry/utils/withProject';
 
 import ComingSoon from './comingSoon';
 
+const renderComingSoon = () => <ComingSoon />;
+
 type Props = {
   /**
    * If children is a function then will be treated as a render prop and
@@ -27,7 +29,7 @@ type Props = {
    *
    * Use `organizations:` or `projects:` prefix strings to specify a feature with context.
    */
-  features: string[];
+  features: string | string[];
   /**
    * The following properties will be set by the HoCs
    */
@@ -80,23 +82,23 @@ type FeatureRenderProps = {
  * call the original children function  but override the `renderDisabled`
  * with another function/component.
  */
-type RenderDisabledProps = FeatureRenderProps & {
+interface RenderDisabledProps extends FeatureRenderProps {
   children: React.ReactNode | ChildrenRenderFn;
   renderDisabled?: (props: FeatureRenderProps) => React.ReactNode;
-};
+}
 
 export type RenderDisabledFn = (props: RenderDisabledProps) => React.ReactNode;
 
-type ChildRenderProps = FeatureRenderProps & {
+interface ChildRenderProps extends FeatureRenderProps {
   renderDisabled?: undefined | boolean | RenderDisabledFn;
-};
+}
 
 export type ChildrenRenderFn = (props: ChildRenderProps) => React.ReactNode;
 
 type AllFeatures = {
-  configFeatures: string[];
-  organization: string[];
-  project: string[];
+  configFeatures: ReadonlyArray<string>;
+  organization: ReadonlyArray<string>;
+  project: ReadonlyArray<string>;
 };
 
 /**
@@ -119,9 +121,6 @@ class Feature extends Component<Props> {
   }
 
   hasFeature(feature: string, features: AllFeatures) {
-    const shouldMatchOnlyProject = feature.match(/^projects:(.+)/);
-    const shouldMatchOnlyOrg = feature.match(/^organizations:(.+)/);
-
     // Array of feature strings
     const {configFeatures, organization, project} = features;
 
@@ -131,10 +130,12 @@ class Feature extends Component<Props> {
       return true;
     }
 
+    const shouldMatchOnlyProject = feature.match(/^projects:(.+)/);
     if (shouldMatchOnlyProject) {
       return project.includes(shouldMatchOnlyProject[1]);
     }
 
+    const shouldMatchOnlyOrg = feature.match(/^organizations:(.+)/);
     if (shouldMatchOnlyOrg) {
       return organization.includes(shouldMatchOnlyOrg[1]);
     }
@@ -157,7 +158,10 @@ class Feature extends Component<Props> {
     const allFeatures = this.getAllFeatures();
     const method = requireAll ? 'every' : 'some';
     const hasFeature =
-      !features || features[method](feat => this.hasFeature(feat, allFeatures));
+      !features ||
+      (typeof features === 'string'
+        ? this.hasFeature(features, allFeatures)
+        : features[method](feat => this.hasFeature(feat, allFeatures)));
 
     // Default renderDisabled to the ComingSoon component
     let customDisabledRender =
@@ -165,7 +169,7 @@ class Feature extends Component<Props> {
         ? false
         : typeof renderDisabled === 'function'
         ? renderDisabled
-        : () => <ComingSoon />;
+        : renderComingSoon;
 
     // Override the renderDisabled function with a hook store function if there
     // is one registered for the feature.
@@ -179,7 +183,7 @@ class Feature extends Component<Props> {
     const renderProps = {
       organization,
       project,
-      features,
+      features: Array.isArray(features) ? features : [features],
       hasFeature,
     };
 

+ 15 - 4
static/app/components/acl/featureDisabled.spec.tsx

@@ -4,9 +4,20 @@ import FeatureDisabled from 'sentry/components/acl/featureDisabled';
 
 describe('FeatureDisabled', function () {
   it('renders', function () {
+    render(
+      <FeatureDisabled features="organization:my-features" featureName="Some Feature" />
+    );
+
+    expect(
+      screen.getByText('This feature is not enabled on your Sentry installation.')
+    ).toBeInTheDocument();
+    expect(screen.getByText('Help')).toBeInTheDocument();
+  });
+
+  it('supports a list of disabled features', function () {
     render(
       <FeatureDisabled
-        features={['organization:my-features']}
+        features={['organization:my-features', 'organization:other-feature']}
         featureName="Some Feature"
       />
     );
@@ -22,7 +33,7 @@ describe('FeatureDisabled', function () {
     render(
       <FeatureDisabled
         message={customMessage}
-        features={['organization:my-features']}
+        features="organization:my-features"
         featureName="Some Feature"
       />
     );
@@ -35,7 +46,7 @@ describe('FeatureDisabled', function () {
     render(
       <FeatureDisabled
         alert={customAlert}
-        features={['organization:my-features']}
+        features="organization:my-features"
         featureName="Some Feature"
       />
     );
@@ -46,7 +57,7 @@ describe('FeatureDisabled', function () {
     render(
       <FeatureDisabled
         alert
-        features={['organization:my-features']}
+        features="organization:my-features"
         featureName="Some Feature"
       />
     );

+ 6 - 3
static/app/components/acl/featureDisabled.tsx

@@ -11,10 +11,13 @@ import {space} from 'sentry/styles/space';
 import {selectText} from 'sentry/utils/selectText';
 import useCopyToClipboard from 'sentry/utils/useCopyToClipboard';
 
-const installText = (features: string[], featureName: string): string =>
-  `# ${t('Enables the %s feature', featureName)}\n${features
+const installText = (features: Props['features'], featureName: string): string => {
+  const featuresList = Array.isArray(features) ? features : [features];
+
+  return `# ${t('Enables the %s feature', featureName)}\n${featuresList
     .map(f => `SENTRY_FEATURES['${f}'] = True`)
     .join('\n')}`;
+};
 
 type Props = {
   /**
@@ -26,7 +29,7 @@ type Props = {
    * The feature flag keys that should be displayed in the code example for
    * enabling the feature.
    */
-  features: string[];
+  features: string | string[];
   /**
    * Render the disabled message within a warning Alert. A custom Alert
    * component may be provided.

+ 1 - 1
static/app/components/acl/featureDisabledModal.spec.tsx

@@ -20,7 +20,7 @@ describe('FeatureTourModal', function () {
         closeModal={onCloseModal}
         CloseButton={() => <button>Close</button>}
         featureName="Default Feature"
-        features={['organization:test-feature']}
+        features="organization:test-feature"
         {...props}
       />
     );

+ 1 - 1
static/app/components/acl/featureDisabledModal.tsx

@@ -7,7 +7,7 @@ import {t} from 'sentry/locale';
 
 type Props = ModalRenderProps & {
   featureName: string;
-  features: string[];
+  features: string | string[];
   message?: string;
 };
 

+ 1 - 1
static/app/components/dataExport.tsx

@@ -105,7 +105,7 @@ function DataExport({
   }, [payload.queryInfo, payload.queryType, organization.slug, api]);
 
   return (
-    <Feature features={['organizations:discover-query']}>
+    <Feature features="organizations:discover-query">
       {inProgress ? (
         <Button
           size="sm"

+ 1 - 1
static/app/components/discover/discoverFeature.tsx

@@ -32,7 +32,7 @@ function DiscoverFeature({children}: Props) {
   return (
     <Feature
       hookName="feature-disabled:open-discover"
-      features={['organizations:discover-basic']}
+      features="organizations:discover-basic"
       renderDisabled={renderDisabled}
     >
       {({hasFeature}) => children({hasFeature})}

+ 1 - 1
static/app/components/dynamicSampling/investigationRule.tsx

@@ -277,7 +277,7 @@ function InvestigationRuleCreationInternal(props: PropsInternal) {
 export function InvestigationRuleCreation(props: Props) {
   const organization = useOrganization();
   return (
-    <Feature features={['investigation-bias']}>
+    <Feature features="investigation-bias">
       <InvestigationRuleCreationInternal {...props} organization={organization} />
     </Feature>
   );

+ 1 - 1
static/app/components/events/contexts/profile/index.tsx

@@ -26,7 +26,7 @@ export function ProfileEventContext({event, data}: ProfileContextProps) {
   const meta = event._meta?.contexts?.profile ?? {};
 
   return (
-    <Feature organization={organization} features={['profiling']}>
+    <Feature organization={organization} features="profiling">
       <ErrorBoundary mini>
         <KeyValueList
           data={getKnownData<ProfileContext, ProfileContextKey>({

Некоторые файлы не были показаны из-за большого количества измененных файлов