Browse Source

feat(codeowners): Rm CodeOwners CTA hooks (#33593)

Updates the CTA in Issue Details page with more complex states. I also expanded the audience to include free plan users to see if we can get some conversions from this feature.
NisanthanNanthakumar 2 years ago
parent
commit
f1c0babaef

+ 22 - 16
static/app/components/group/suggestedOwners/ownershipRules.tsx

@@ -7,7 +7,7 @@ import Access from 'sentry/components/acl/access';
 import GuideAnchor from 'sentry/components/assistant/guideAnchor';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
-import FeatureBadge from 'sentry/components/featureBadge';
+import HookOrDefault from 'sentry/components/hookOrDefault';
 import {Hovercard} from 'sentry/components/hovercard';
 import {Panel} from 'sentry/components/panels';
 import {IconClose, IconQuestion} from 'sentry/icons';
@@ -27,6 +27,26 @@ type Props = {
   project: Project;
 };
 
+const CodeOwnersCTA = HookOrDefault({
+  hookName: 'component:codeowners-cta',
+  defaultComponent: ({organization, project}) => (
+    <SetupButton
+      size="xsmall"
+      priority="primary"
+      to={`/settings/${organization.slug}/projects/${project.slug}/ownership/`}
+      onClick={() =>
+        trackIntegrationAnalytics('integrations.code_owners_cta_setup_clicked', {
+          view: 'stacktrace_issue_details',
+          project_id: project.id,
+          organization,
+        })
+      }
+    >
+      {t('Setup')}
+    </SetupButton>
+  ),
+});
+
 const OwnershipRules = ({
   project,
   organization,
@@ -62,7 +82,6 @@ const OwnershipRules = ({
     <Container dashedBorder>
       <HeaderContainer>
         <Header>{t('Codeowners sync')}</Header>{' '}
-        <FeatureBadge style={{top: -3}} type="new" noTooltip />
         <DismissButton
           icon={<IconClose size="xs" />}
           priority="link"
@@ -76,20 +95,7 @@ const OwnershipRules = ({
         )}
       </Content>
       <ButtonBar gap={1}>
-        <SetupButton
-          size="xsmall"
-          priority="primary"
-          to={`/settings/${organization.slug}/projects/${project.slug}/ownership/`}
-          onClick={() =>
-            trackIntegrationAnalytics('integrations.code_owners_cta_setup_clicked', {
-              view: 'stacktrace_issue_details',
-              project_id: project.id,
-              organization,
-            })
-          }
-        >
-          {t('Setup')}
-        </SetupButton>
+        <CodeOwnersCTA organization={organization} project={project} />
         <Button
           size="xsmall"
           external

+ 1 - 21
static/app/components/group/suggestedOwners/suggestedOwners.tsx

@@ -4,15 +4,7 @@ import {assignToActor, assignToUser} from 'sentry/actionCreators/group';
 import {promptsCheck, promptsUpdate} from 'sentry/actionCreators/prompts';
 import {Client} from 'sentry/api';
 import AsyncComponent from 'sentry/components/asyncComponent';
-import {
-  Actor,
-  CodeOwner,
-  Committer,
-  Group,
-  Organization,
-  Project,
-  RepositoryProjectPathConfig,
-} from 'sentry/types';
+import {Actor, CodeOwner, Committer, Group, Organization, Project} from 'sentry/types';
 import {Event} from 'sentry/types/event';
 import {trackIntegrationAnalytics} from 'sentry/utils/integrationUtil';
 import {promptIsDismissed} from 'sentry/utils/promptIsDismissed';
@@ -36,7 +28,6 @@ type Props = {
 } & AsyncComponent['props'];
 
 type State = {
-  codeMappings: RepositoryProjectPathConfig[];
   codeowners: CodeOwner[];
   event_owners: {owners: Array<Actor>; rules: Rules};
   isDismissed: boolean;
@@ -49,7 +40,6 @@ class SuggestedOwners extends AsyncComponent<Props, State> {
       event: {rules: [], owners: []},
       codeowners: [],
       isDismissed: true,
-      codeMappings: [],
     };
   }
 
@@ -60,11 +50,6 @@ class SuggestedOwners extends AsyncComponent<Props, State> {
         'event_owners',
         `/projects/${organization.slug}/${project.slug}/events/${event.id}/owners/`,
       ],
-      [
-        'codeMappings',
-        `/organizations/${organization.slug}/code-mappings/`,
-        {query: {project: -1}},
-      ],
     ];
     if (organization.features.includes('integrations-codeowners')) {
       endpoints.push([
@@ -97,12 +82,7 @@ class SuggestedOwners extends AsyncComponent<Props, State> {
 
   async checkCodeOwnersPrompt() {
     const {api, organization, project} = this.props;
-    const {codeMappings} = this.state;
 
-    // Show CTA to all orgs that have Stack Trace Linking setup.
-    if (!codeMappings.length) {
-      return;
-    }
     this.setState({loading: true});
     // check our prompt backend
     const promptData = await promptsCheck(api, {

+ 7 - 6
static/app/types/hooks.tsx

@@ -70,11 +70,6 @@ type DisabledMemberTooltipProps = {children: React.ReactNode};
 
 type DashboardHeadersProps = {organization: Organization};
 
-type CodeOwnersHeaderProps = {
-  addCodeOwner: () => void;
-  handleRequest: () => void;
-};
-
 type FirstPartyIntegrationAlertProps = {
   integrations: Integration[];
   hideCTA?: boolean;
@@ -87,11 +82,17 @@ type FirstPartyIntegrationAdditionalCTAProps = {
 
 type GuideUpdateCallback = (nextGuide: Guide | null, opts: {dismissed?: boolean}) => void;
 
+type CodeOwnersCTAProps = {
+  organization: Organization;
+  project: Project;
+  addCodeOwner?: () => void;
+  handleRequest?: () => void;
+};
 /**
  * Component wrapping hooks
  */
 export type ComponentHooks = {
-  'component:codeowners-header': () => React.ComponentType<CodeOwnersHeaderProps>;
+  'component:codeowners-cta': () => React.ComponentType<CodeOwnersCTAProps>;
   'component:dashboards-header': () => React.ComponentType<DashboardHeadersProps>;
   'component:disabled-app-store-connect-multiple': () => React.ComponentType<DisabledAppStoreConnectMultiple>;
   'component:disabled-custom-symbol-sources': () => React.ComponentType<DisabledCustomSymbolSources>;

+ 0 - 1
static/app/views/settings/project/navigationConfiguration.tsx

@@ -50,7 +50,6 @@ export default function getConfiguration({
           path: `${pathPrefix}/ownership/`,
           title: t('Issue Owners'),
           description: t('Manage issue ownership rules for a project'),
-          badge: () => 'new',
         },
         {
           path: `${pathPrefix}/data-forwarding/`,

+ 79 - 48
static/app/views/settings/project/projectOwnership/addCodeOwnerModal.tsx

@@ -1,10 +1,10 @@
-import {Component, Fragment} from 'react';
+import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {ModalRenderProps} from 'sentry/actionCreators/modal';
-import {Client} from 'sentry/api';
 import Alert from 'sentry/components/alert';
+import AsyncComponent from 'sentry/components/asyncComponent';
 import Button from 'sentry/components/button';
 import Form from 'sentry/components/forms/form';
 import SelectField from 'sentry/components/forms/selectField';
@@ -23,33 +23,52 @@ import {
   RepositoryProjectPathConfig,
 } from 'sentry/types';
 import {getIntegrationIcon} from 'sentry/utils/integrationUtil';
-import withApi from 'sentry/utils/withApi';
 
 type Props = {
-  api: Client;
-  codeMappings: RepositoryProjectPathConfig[];
-  integrations: Integration[];
-  onSave: (data: CodeOwner) => void;
   organization: Organization;
   project: Project;
-} & ModalRenderProps;
+  onSave?: (data: CodeOwner) => void;
+} & ModalRenderProps &
+  AsyncComponent['props'];
 
 type State = {
   codeMappingId: string | null;
+  codeMappings: RepositoryProjectPathConfig[];
   codeownersFile: CodeownersFile | null;
   error: boolean;
   errorJSON: {raw?: string} | null;
+  integrations: Integration[];
   isLoading: boolean;
-};
+} & AsyncComponent['state'];
 
-class AddCodeOwnerModal extends Component<Props, State> {
-  state: State = {
-    codeownersFile: null,
-    codeMappingId: null,
-    isLoading: false,
-    error: false,
-    errorJSON: null,
-  };
+class AddCodeOwnerModal extends AsyncComponent<Props, State> {
+  getDefaultState() {
+    return {
+      ...super.getDefaultState(),
+      codeownersFile: null,
+      codeMappingId: null,
+      isLoading: false,
+      error: false,
+      errorJSON: null,
+    };
+  }
+
+  getEndpoints(): ReturnType<AsyncComponent['getEndpoints']> {
+    const {organization, project} = this.props;
+    const endpoints: ReturnType<AsyncComponent['getEndpoints']> = [
+      [
+        'codeMappings',
+        `/organizations/${organization.slug}/code-mappings/`,
+        {query: {project: project.id}},
+      ],
+      [
+        'integrations',
+        `/organizations/${organization.slug}/integrations/`,
+        {query: {features: ['codeowners']}},
+      ],
+    ];
+    return endpoints;
+  }
 
   fetchFile = async (codeMappingId: string) => {
     const {organization} = this.props;
@@ -61,7 +80,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
       isLoading: true,
     });
     try {
-      const data: CodeownersFile = await this.props.api.requestPromise(
+      const data: CodeownersFile = await this.api.requestPromise(
         `/organizations/${organization.slug}/code-mappings/${codeMappingId}/codeowners/`,
         {
           method: 'GET',
@@ -74,8 +93,8 @@ class AddCodeOwnerModal extends Component<Props, State> {
   };
 
   addFile = async () => {
-    const {organization, project, codeMappings} = this.props;
-    const {codeownersFile, codeMappingId} = this.state;
+    const {organization, project} = this.props;
+    const {codeownersFile, codeMappingId, codeMappings} = this.state;
 
     if (codeownersFile) {
       const postData: {
@@ -87,7 +106,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
       };
 
       try {
-        const data = await this.props.api.requestPromise(
+        const data = await this.api.requestPromise(
           `/projects/${organization.slug}/${project.slug}/codeowners/`,
           {
             method: 'POST',
@@ -109,7 +128,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
   };
 
   handleAddedFile(data: CodeOwner) {
-    this.props.onSave(data);
+    this.props.onSave && this.props.onSave(data);
     this.props.closeModal();
   }
 
@@ -128,8 +147,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
   }
 
   errorMessage(baseUrl) {
-    const {errorJSON, codeMappingId} = this.state;
-    const {codeMappings} = this.props;
+    const {errorJSON, codeMappingId, codeMappings} = this.state;
     const codeMapping = codeMappings.find(mapping => mapping.id === codeMappingId);
     const {integrationId, provider} = codeMapping as RepositoryProjectPathConfig;
     const errActors = errorJSON?.raw?.[0].split('\n').map((el, i) => <p key={i}>{el}</p>);
@@ -189,37 +207,50 @@ class AddCodeOwnerModal extends Component<Props, State> {
     );
   }
 
-  render() {
+  renderBody() {
     const {Header, Body, Footer} = this.props;
-    const {codeownersFile, error, errorJSON} = this.state;
-    const {codeMappings, integrations, organization} = this.props;
+    const {codeownersFile, error, errorJSON, codeMappings, integrations} = this.state;
+    const {organization} = this.props;
     const baseUrl = `/settings/${organization.slug}/integrations`;
 
     return (
       <Fragment>
         <Header closeButton>{t('Add Code Owner File')}</Header>
         <Body>
-          {!codeMappings.length && (
-            <Fragment>
-              <div>
-                {t(
-                  "Configure code mapping to add your CODEOWNERS file. Select the integration you'd like to use for mapping:"
-                )}
-              </div>
-              <IntegrationsList>
-                {integrations.map(integration => (
-                  <Button
-                    key={integration.id}
-                    type="button"
-                    to={`${baseUrl}/${integration.provider.key}/${integration.id}/?tab=codeMappings&referrer=add-codeowners`}
-                  >
-                    {getIntegrationIcon(integration.provider.key)}
-                    <IntegrationName>{integration.name}</IntegrationName>
+          {!codeMappings.length ? (
+            !integrations.length ? (
+              <Fragment>
+                <div>
+                  {t('Install a GitHub or GitLab integration to use this feature.')}
+                </div>
+                <Container style={{paddingTop: space(2)}}>
+                  <Button type="button" priority="primary" size="small" to={baseUrl}>
+                    Setup Integration
                   </Button>
-                ))}
-              </IntegrationsList>
-            </Fragment>
-          )}
+                </Container>
+              </Fragment>
+            ) : (
+              <Fragment>
+                <div>
+                  {t(
+                    "Configure code mapping to add your CODEOWNERS file. Select the integration you'd like to use for mapping:"
+                  )}
+                </div>
+                <IntegrationsList>
+                  {integrations.map(integration => (
+                    <Button
+                      key={integration.id}
+                      type="button"
+                      to={`${baseUrl}/${integration.provider.key}/${integration.id}/?tab=codeMappings&referrer=add-codeowners`}
+                    >
+                      {getIntegrationIcon(integration.provider.key)}
+                      <IntegrationName>{integration.name}</IntegrationName>
+                    </Button>
+                  ))}
+                </IntegrationsList>
+              </Fragment>
+            )
+          ) : null}
           {codeMappings.length > 0 && (
             <Form
               apiMethod="POST"
@@ -263,7 +294,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
   }
 }
 
-export default withApi(AddCodeOwnerModal);
+export default AddCodeOwnerModal;
 export {AddCodeOwnerModal};
 
 const StyledSelectField = styled(SelectField)`

+ 1 - 32
static/app/views/settings/project/projectOwnership/index.tsx

@@ -16,17 +16,10 @@ import Button from 'sentry/components/button';
 import FeatureBadge from 'sentry/components/featureBadge';
 import Form from 'sentry/components/forms/form';
 import JsonForm from 'sentry/components/forms/jsonForm';
-import HookOrDefault from 'sentry/components/hookOrDefault';
 import ExternalLink from 'sentry/components/links/externalLink';
 import {t, tct} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {
-  CodeOwner,
-  Integration,
-  Organization,
-  Project,
-  RepositoryProjectPathConfig,
-} from 'sentry/types';
+import {CodeOwner, Organization, Project} from 'sentry/types';
 import routeTitleGen from 'sentry/utils/routeTitle';
 import AsyncView from 'sentry/views/asyncView';
 import FeedbackAlert from 'sentry/views/settings/account/notifications/feedbackAlert';
@@ -42,17 +35,10 @@ type Props = {
 } & RouteComponentProps<{orgId: string; projectId: string}, {}>;
 
 type State = {
-  codeMappings: RepositoryProjectPathConfig[];
-  integrations: Integration[];
   ownership: null | any;
   codeowners?: CodeOwner[];
 } & AsyncView['state'];
 
-const CodeOwnersHeader = HookOrDefault({
-  hookName: 'component:codeowners-header',
-  defaultComponent: () => <Fragment />,
-});
-
 class ProjectOwnership extends AsyncView<Props, State> {
   getTitle() {
     const {project} = this.props;
@@ -63,16 +49,6 @@ class ProjectOwnership extends AsyncView<Props, State> {
     const {organization, project} = this.props;
     const endpoints: ReturnType<AsyncView['getEndpoints']> = [
       ['ownership', `/projects/${organization.slug}/${project.slug}/ownership/`],
-      [
-        'codeMappings',
-        `/organizations/${organization.slug}/code-mappings/`,
-        {query: {project: project.id}},
-      ],
-      [
-        'integrations',
-        `/organizations/${organization.slug}/integrations/`,
-        {query: {features: ['codeowners']}},
-      ],
     ];
     if (organization.features.includes('integrations-codeowners')) {
       endpoints.push([
@@ -85,14 +61,11 @@ class ProjectOwnership extends AsyncView<Props, State> {
   }
 
   handleAddCodeOwner = () => {
-    const {codeMappings, integrations} = this.state;
     openModal(modalProps => (
       <AddCodeOwnerModal
         {...modalProps}
         organization={this.props.organization}
         project={this.props.project}
-        codeMappings={codeMappings}
-        integrations={integrations}
         onSave={this.handleCodeOwnerAdded}
       />
     ));
@@ -332,10 +305,6 @@ tags.sku_class:enterprise #enterprise`;
           }
         />
         <IssueOwnerDetails>{this.getDetail()}</IssueOwnerDetails>
-        <CodeOwnersHeader
-          addCodeOwner={this.handleAddCodeOwner}
-          handleRequest={this.handleAddCodeOwnerRequest}
-        />
 
         <PermissionAlert />
         <FeedbackAlert />

+ 0 - 2
static/app/views/settings/project/projectOwnership/rulesPanel.tsx

@@ -2,7 +2,6 @@ import TextareaAutosize from 'react-autosize-textarea';
 import styled from '@emotion/styled';
 import moment from 'moment';
 
-import FeatureBadge from 'sentry/components/featureBadge';
 import {Panel, PanelBody, PanelHeader} from 'sentry/components/panels';
 import {IconGithub, IconGitlab, IconSentry} from 'sentry/icons';
 import {inputStyles} from 'sentry/styles/input';
@@ -59,7 +58,6 @@ function RulesPanel({
             {renderIcon()}
             <Title>{renderTitle()}</Title>
             {repoName && <Repository>{`- ${repoName}`}</Repository>}
-            <FeatureBadge type="new" />
           </Container>,
           <Container key="control">
             <SyncDate>

+ 16 - 32
tests/js/spec/views/addCodeOwnerModal.spec.jsx

@@ -27,18 +27,23 @@ describe('AddCodeOwnerModal', function () {
 
   beforeEach(function () {
     Client.clearMockResponses();
+    Client.addMockResponse({
+      url: `/organizations/${org.slug}/code-mappings/`,
+      method: 'GET',
+      query: {project: '2'},
+      body: [codeMapping],
+    });
+    Client.addMockResponse({
+      url: `/organizations/${org.slug}/integrations/`,
+      method: 'GET',
+      query: {features: ['codeowners']},
+      body: [integration],
+    });
   });
 
   it('renders', function () {
     const wrapper = mountWithTheme(
-      <AddCodeOwnerModal
-        {...modalProps}
-        api={new Client()}
-        organization={org}
-        project={project}
-        codeMappings={[codeMapping]}
-        onSave={() => {}}
-      />
+      <AddCodeOwnerModal {...modalProps} organization={org} project={project} />
     );
     expect(wrapper.find('Button').prop('disabled')).toBe(true);
   });
@@ -50,14 +55,7 @@ describe('AddCodeOwnerModal', function () {
       body: {html_url: 'blah', filepath: 'CODEOWNERS', raw: '* @MeredithAnya\n'},
     });
     const wrapper = mountWithTheme(
-      <AddCodeOwnerModal
-        {...modalProps}
-        api={new Client()}
-        organization={org}
-        project={project}
-        codeMappings={[codeMapping]}
-        onSave={() => {}}
-      />
+      <AddCodeOwnerModal {...modalProps} organization={org} project={project} />
     );
 
     selectByValue(wrapper, codeMapping.id, {name: 'codeMappingId'});
@@ -77,14 +75,7 @@ describe('AddCodeOwnerModal', function () {
       statusCode: 404,
     });
     const wrapper = mountWithTheme(
-      <AddCodeOwnerModal
-        {...modalProps}
-        api={new Client()}
-        organization={org}
-        project={project}
-        codeMappings={[codeMapping]}
-        onSave={() => {}}
-      />
+      <AddCodeOwnerModal {...modalProps} organization={org} project={project} />
     );
 
     selectByValue(wrapper, codeMapping.id, {name: 'codeMappingId'});
@@ -108,14 +99,7 @@ describe('AddCodeOwnerModal', function () {
       body: {},
     });
     const wrapper = mountWithTheme(
-      <AddCodeOwnerModal
-        {...modalProps}
-        api={new Client()}
-        organization={org}
-        project={project}
-        codeMappings={[codeMapping]}
-        onSave={() => {}}
-      />
+      <AddCodeOwnerModal {...modalProps} organization={org} project={project} />
     );
 
     selectByValue(wrapper, codeMapping.id, {name: 'codeMappingId'});