feat(codeowners): Fix up Add Codeowners modal states (#25645)

Objective
Preview file should open in new tab, not in current tab. There should be an empty state screen for users who click Add CodeOwners and have a source code integration to let users setup a Code Mapping.

src/sentry/api/endpoints/organization_integrations.py

@@ -8,6 +8,10 @@ class OrganizationIntegrationsEndpoint(OrganizationEndpoint):
     permission_classes = (OrganizationIntegrationsPermission,)
 
     def get(self, request, organization):
+
+        # filter by integration provider features
+        features = [feature.lower() for feature in request.GET.getlist("features", [])]
+
         integrations = OrganizationIntegration.objects.filter(
             organization=organization, status=ObjectStatus.VISIBLE
         )
@@ -20,10 +24,34 @@ class OrganizationIntegrationsEndpoint(OrganizationEndpoint):
         if request.GET.get("includeConfig") == "0":
             include_config = False
 
+        def on_results(results):
+            if len(features):
+                return [
+                    serialize(
+                        i,
+                        request.user,
+                        include_config=include_config,
+                    )
+                    for i in filter(
+                        # check if any feature in query param is in the provider feature list
+                        lambda i: any(
+                            f
+                            in [
+                                feature.name.lower()
+                                for feature in list(i.integration.get_provider().features)
+                            ]
+                            for f in features
+                        ),
+                        results,
+                    )
+                ]
+
+            return serialize(results, request.user, include_config=include_config)
+
         return self.paginate(
             queryset=integrations,
             request=request,
             order_by="integration__name",
-            on_results=lambda x: serialize(x, request.user, include_config=include_config),
+            on_results=on_results,
             paginator_cls=OffsetPaginator,
         )

src/sentry/integrations/base.py

@@ -95,6 +95,7 @@ class IntegrationFeatures(Enum):
     SERVERLESS = "serverless"
     TICKET_RULES = "ticket-rules"
     STACKTRACE_LINK = "stacktrace-link"
+    CODEOWNERS = "codeowners"
 
     # features currently only existing on plugins:
     DATA_FORWARDING = "data-forwarding"

src/sentry/integrations/github/integration.py

@@ -172,6 +172,7 @@ class GitHubIntegrationProvider(IntegrationProvider):
             IntegrationFeatures.COMMITS,
             IntegrationFeatures.ISSUE_BASIC,
             IntegrationFeatures.STACKTRACE_LINK,
+            IntegrationFeatures.CODEOWNERS,
         ]
     )
 

src/sentry/integrations/gitlab/integration.py

@@ -273,6 +273,7 @@ class GitlabIntegrationProvider(IntegrationProvider):
             IntegrationFeatures.ISSUE_BASIC,
             IntegrationFeatures.COMMITS,
             IntegrationFeatures.STACKTRACE_LINK,
+            IntegrationFeatures.CODEOWNERS,
         ]
     )
 

static/app/views/settings/organizationIntegrations/configureIntegration.tsx

@@ -54,7 +54,11 @@ class ConfigureIntegration extends AsyncView<Props, State> {
 
   componentDidMount() {
     const {location} = this.props;
-    const value = location.query.tab === 'codeMappings' ? 'codeMappings' : 'repos';
+    const value =
+      (['codeMappings', 'userMappings', 'teamMappings'] as const).find(
+        tab => tab === location.query.tab
+      ) || 'repos';
+
     // eslint-disable-next-line react/no-did-mount-set-state
     this.setState({tab: value});
   }

static/app/views/settings/project/projectOwnership/addCodeOwnerModal.tsx

@@ -5,11 +5,20 @@ import {ModalRenderProps} from 'app/actionCreators/modal';
 import {Client} from 'app/api';
 import Alert from 'app/components/alert';
 import Button from 'app/components/button';
+import Link from 'app/components/links/link';
 import LoadingIndicator from 'app/components/loadingIndicator';
 import {Panel, PanelBody} from 'app/components/panels';
 import {IconCheckmark, IconNot} from 'app/icons';
-import {t} from 'app/locale';
-import {CodeOwners, Organization, Project, RepositoryProjectPathConfig} from 'app/types';
+import {t, tct} from 'app/locale';
+import space from 'app/styles/space';
+import {
+  CodeOwners,
+  Integration,
+  Organization,
+  Project,
+  RepositoryProjectPathConfig,
+} from 'app/types';
+import {getIntegrationIcon} from 'app/utils/integrationUtil';
 import withApi from 'app/utils/withApi';
 import Form from 'app/views/settings/components/forms/form';
 import SelectField from 'app/views/settings/components/forms/selectField';
@@ -19,12 +28,13 @@ type Props = {
   organization: Organization;
   project: Project;
   codeMappings: RepositoryProjectPathConfig[];
+  integrations: Integration[];
   onSave: (data: CodeOwners) => void;
 } & ModalRenderProps;
 
 type State = {
   codeownerFile: CodeOwnerFile | null;
-  codeMappingId: number | null;
+  codeMappingId: string | null;
   isLoading: boolean;
   error: boolean;
   errorJSON: {raw?: string} | null;
@@ -45,7 +55,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
     errorJSON: null,
   };
 
-  fetchFile = async (codeMappingId: number) => {
+  fetchFile = async (codeMappingId: string) => {
     const {organization} = this.props;
     this.setState({
       codeMappingId,
@@ -103,7 +113,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
         <SourceFileBody>
           <IconCheckmark size="md" isCircled color="green200" />
           {codeownerFile.filepath}
-          <Button size="small" href={codeownerFile.html_url}>
+          <Button size="small" href={codeownerFile.html_url} target="_blank">
             {t('Preview File')}
           </Button>
         </SourceFileBody>
@@ -111,11 +121,33 @@ class AddCodeOwnerModal extends Component<Props, State> {
     );
   }
 
-  errorMessage() {
-    const {errorJSON} = this.state;
+  errorMessage(baseUrl) {
+    const {errorJSON, codeMappingId} = this.state;
+    const {codeMappings} = this.props;
+    const codeMapping = codeMappings.find(mapping => mapping.id === codeMappingId);
+    const {integrationId, provider} = codeMapping as RepositoryProjectPathConfig;
     return (
       <Alert type="error" icon={<IconNot size="md" />}>
         <p>{errorJSON?.raw?.[0]}</p>
+        {codeMapping && (
+          <p>
+            {tct(
+              'Configure [userMappingsLink:User Mappings] or[teamMappingsLink:Team Mappings] for any missing associations.',
+              {
+                userMappingsLink: (
+                  <Link
+                    to={`${baseUrl}/${provider?.key}/${integrationId}/?tab=userMappings&referrer=add-codeowners`}
+                  />
+                ),
+                teamMappingsLink: (
+                  <Link
+                    to={`${baseUrl}/${provider?.key}/${integrationId}/?tab=teamMappings&referrer=add-codeowners`}
+                  />
+                ),
+              }
+            )}
+          </p>
+        )}
       </Alert>
     );
   }
@@ -124,11 +156,9 @@ class AddCodeOwnerModal extends Component<Props, State> {
     const {codeMappingId, isLoading} = this.state;
     if (isLoading) {
       return (
-        <Panel>
-          <NoSourceFileBody>
-            <LoadingIndicator mini />
-          </NoSourceFileBody>
-        </Panel>
+        <Container>
+          <LoadingIndicator mini />
+        </Container>
       );
     }
     if (!codeMappingId) {
@@ -151,11 +181,34 @@ class AddCodeOwnerModal extends Component<Props, State> {
   render() {
     const {Header, Body, Footer} = this.props;
     const {codeownerFile, error, errorJSON} = this.state;
-    const {codeMappings} = this.props;
+    const {codeMappings, integrations, 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>
+                  </Button>
+                ))}
+              </IntegrationsList>
+            </Fragment>
+          )}
           {codeMappings.length > 0 && (
             <Form
               apiMethod="POST"
@@ -179,7 +232,7 @@ class AddCodeOwnerModal extends Component<Props, State> {
 
               <FileResult>
                 {codeownerFile ? this.sourceFile(codeownerFile) : this.noSourceFile()}
-                {error && errorJSON && this.errorMessage()}
+                {error && errorJSON && this.errorMessage(baseUrl)}
               </FileResult>
             </Form>
           )}
@@ -213,13 +266,27 @@ const NoSourceFileBody = styled(PanelBody)`
   display: grid;
   padding: 12px;
   grid-template-columns: 30px 1fr;
-  align-items: flex-start;
-  min-height: 150px;
+  align-items: center;
 `;
 const SourceFileBody = styled(PanelBody)`
   display: grid;
   padding: 12px;
   grid-template-columns: 30px 1fr 100px;
-  align-items: flex-start;
-  min-height: 150px;
+  align-items: center;
+`;
+
+const IntegrationsList = styled('div')`
+  display: grid;
+  grid-gap: ${space(1)};
+  justify-items: center;
+  margin-top: ${space(2)};
+`;
+
+const IntegrationName = styled('p')`
+  padding-left: 10px;
+`;
+
+const Container = styled('div')`
+  display: flex;
+  justify-content: center;
 `;

static/app/views/settings/project/projectOwnership/index.tsx

@@ -8,7 +8,13 @@ import Button from 'app/components/button';
 import ExternalLink from 'app/components/links/externalLink';
 import {t, tct} from 'app/locale';
 import space from 'app/styles/space';
-import {CodeOwners, Organization, Project, RepositoryProjectPathConfig} from 'app/types';
+import {
+  CodeOwners,
+  Integration,
+  Organization,
+  Project,
+  RepositoryProjectPathConfig,
+} from 'app/types';
 import routeTitleGen from 'app/utils/routeTitle';
 import AsyncView from 'app/views/asyncView';
 import Form from 'app/views/settings/components/forms/form';
@@ -28,6 +34,7 @@ type State = {
   ownership: null | any;
   codeMappings: RepositoryProjectPathConfig[];
   codeowners?: CodeOwners[];
+  integrations: Integration[];
 } & AsyncView['state'];
 
 class ProjectOwnership extends AsyncView<Props, State> {
@@ -42,26 +49,34 @@ class ProjectOwnership extends AsyncView<Props, State> {
       ['ownership', `/projects/${organization.slug}/${project.slug}/ownership/`],
       [
         'codeMappings',
-        `/organizations/${organization.slug}/code-mappings/?projectId=${project.id}`,
+        `/organizations/${organization.slug}/code-mappings/`,
+        {query: {projectId: project.id}},
+      ],
+      [
+        'integrations',
+        `/organizations/${organization.slug}/integrations/`,
+        {query: {features: ['codeowners']}},
       ],
     ];
     if (organization.features.includes('import-codeowners')) {
       endpoints.push([
         'codeowners',
-        `/projects/${organization.slug}/${project.slug}/codeowners/?expand=codeMapping`,
+        `/projects/${organization.slug}/${project.slug}/codeowners/`,
+        {query: {expand: ['codeMapping']}},
       ]);
     }
     return endpoints;
   }
 
   handleAddCodeOwner = () => {
-    const {codeMappings} = this.state;
+    const {codeMappings, integrations} = this.state;
     openModal(modalProps => (
       <AddCodeOwnerModal
         {...modalProps}
         organization={this.props.organization}
         project={this.props.project}
         codeMappings={codeMappings}
+        integrations={integrations}
         onSave={this.handleCodeownerAdded}
       />
     ));

tests/js/sentry-test/fixtures/integrationListDirectory.js

@@ -33,29 +33,49 @@ export function ProviderList() {
   };
 }
 
-export function IntegrationConfig() {
-  return [
-    {
-      accountType: null,
-      configData: {},
-      configOrganization: [],
-      domainName: 'bitbucket.org/%7Bfb715533-bbd7-4666-aa57-01dc93dd9cc0%7D',
-      icon:
-        'https://secure.gravatar.com/avatar/8b4cb68e40b74c90427d8262256bd1c8?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FNN-0.png',
-      id: '4',
-      name: '{fb715533-bbd7-4666-aa57-01dc93dd9cc0}',
-      provider: {
-        aspects: {},
-        canAdd: true,
-        canDisable: false,
-        features: ['commits', 'issue-basic'],
-        key: 'bitbucket',
-        name: 'Bitbucket',
-        slug: 'bitbucket',
-      },
-      status: 'active',
+export function BitbucketIntegrationConfig() {
+  return {
+    accountType: null,
+    configData: {},
+    configOrganization: [],
+    domainName: 'bitbucket.org/%7Bfb715533-bbd7-4666-aa57-01dc93dd9cc0%7D',
+    icon:
+      'https://secure.gravatar.com/avatar/8b4cb68e40b74c90427d8262256bd1c8?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FNN-0.png',
+    id: '4',
+    name: '{fb715533-bbd7-4666-aa57-01dc93dd9cc0}',
+    provider: {
+      aspects: {},
+      canAdd: true,
+      canDisable: false,
+      features: ['commits', 'issue-basic'],
+      key: 'bitbucket',
+      name: 'Bitbucket',
+      slug: 'bitbucket',
     },
-  ];
+    status: 'active',
+  };
+}
+
+export function GithubIntegrationConfig() {
+  return {
+    accountType: null,
+    configData: {},
+    configOrganization: [],
+    domainName: 'github.com',
+    icon: 'https://secure.gravatar.com/avatar/8b4cb68e40b74c90427d8262256bd1c8',
+    id: '5',
+    name: 'NisanthanNanthakumar',
+    provider: {
+      aspects: {},
+      canAdd: true,
+      canDisable: false,
+      features: ['commits', 'issue-basic'],
+      key: 'github',
+      name: 'Github',
+      slug: 'github',
+    },
+    status: 'active',
+  };
 }
 
 export function OrgOwnedApps() {

tests/js/spec/views/organizationIntegrations/integrationListDirectory.spec.jsx

@@ -25,7 +25,10 @@ describe('IntegrationListDirectory', function () {
     beforeEach(() => {
       mockResponse([
         [`/organizations/${org.slug}/config/integrations/`, TestStubs.ProviderList()],
-        [`/organizations/${org.slug}/integrations/`, TestStubs.IntegrationConfig()],
+        [
+          `/organizations/${org.slug}/integrations/`,
+          [TestStubs.BitbucketIntegrationConfig()],
+        ],
         [`/organizations/${org.slug}/sentry-apps/`, TestStubs.OrgOwnedApps()],
         ['/sentry-apps/', TestStubs.PublishedApps()],
         [

tests/js/spec/views/projectOwnership.spec.jsx

@@ -7,35 +7,40 @@ import ProjectOwnership from 'app/views/settings/project/projectOwnership';
 jest.mock('app/actionCreators/modal');
 
 describe('Project Ownership', function () {
-  let org;
-  let project;
+  let org = TestStubs.Organization();
+  const project = TestStubs.ProjectDetails();
 
   beforeEach(function () {
-    org = TestStubs.Organization();
-    project = TestStubs.ProjectDetails();
-
-    Client.addMockResponse({
-      url: `/projects/${org.slug}/${project.slug}/`,
-      method: 'GET',
-      body: project,
-    });
+    Client.clearMockResponses();
     Client.addMockResponse({
       url: `/projects/${org.slug}/${project.slug}/ownership/`,
       method: 'GET',
       body: {
-        raw: 'url:src @dummy@example.com',
-        fallthrough: 'false',
-        autoAssignment: 'false',
+        fallthrough: false,
+        autoAssignment: false,
       },
     });
     Client.addMockResponse({
-      url: `/organizations/${org.slug}/code-mappings/?projectId=${project.id}`,
+      url: `/organizations/${org.slug}/code-mappings/`,
+      query: {projectId: project.id},
+      method: 'GET',
+      body: [],
+    });
+    Client.addMockResponse({
+      url: `/organizations/${org.slug}/integrations/`,
+      query: {features: 'codeowners'},
+      method: 'GET',
+      body: [TestStubs.GithubIntegrationConfig()],
+    });
+    Client.addMockResponse({
+      url: `/projects/${org.slug}/${project.slug}/codeowners/`,
+      features: {expand: 'codeMapping'},
       method: 'GET',
       body: [],
     });
   });
 
-  describe('render()', function () {
+  describe('without codeowners', function () {
     it('renders', function () {
       const wrapper = mountWithTheme(
         <ProjectOwnership
@@ -50,42 +55,11 @@ describe('Project Ownership', function () {
       expect(wrapper.find('CodeOwnerButton').exists()).toBe(false);
     });
   });
-});
-
-describe('Add Codeowner File', function () {
-  const org = TestStubs.Organization({features: ['import-codeowners']});
-  const project = TestStubs.ProjectDetails();
-
-  beforeEach(function () {
-    Client.clearMockResponses();
-    Client.addMockResponse({
-      url: `/projects/${org.slug}/${project.slug}/`,
-      method: 'GET',
-      body: project,
-    });
-    Client.addMockResponse({
-      url: `/projects/${org.slug}/${project.slug}/ownership/`,
-      method: 'GET',
-      body: {
-        raw: 'url:src @dummy@example.com',
-        fallthrough: 'false',
-        autoAssignment: 'false',
-      },
-    });
-    Client.addMockResponse({
-      url: `/organizations/${org.slug}/code-mappings/?projectId=${project.id}`,
-      method: 'GET',
-      body: [],
-    });
-    Client.addMockResponse({
-      url: `/projects/${org.slug}/${project.slug}/codeowners/?expand=codeMapping`,
-      method: 'GET',
-      body: [],
-    });
-  });
 
   describe('codeowner action button', function () {
     it('renders button', function () {
+      org = TestStubs.Organization({features: ['import-codeowners']});
+
       const wrapper = mountWithTheme(
         <ProjectOwnership
           params={{orgId: org.slug, projectId: project.slug}}
@@ -94,6 +68,7 @@ describe('Add Codeowner File', function () {
         />,
         TestStubs.routerContext([{organization: org}])
       );
+
       expect(wrapper.find('CodeOwnerButton').exists()).toBe(true);
     });
     it('clicking button opens modal', async function () {