Browse Source

feat(custom-scm):Add ability to add and edit repositories (#25801)

* feat(custom-sc): Custom source code integration

* update form

* rename

* feat(custom-scm):Add ability to add and edit repositories

* smol edits

* some more tests

* gate api with feature flag

* feature flag frontend and moar tests

* more smol edits

* update tests

* dont need that
MeredithAnya 3 years ago
parent
commit
48f8561dc8

+ 12 - 0
src/sentry/api/endpoints/organization_repository_details.py

@@ -5,6 +5,7 @@ from django.db import transaction
 from rest_framework import serializers
 from rest_framework.response import Response
 
+from sentry import features
 from sentry.api.bases.organization import OrganizationEndpoint, OrganizationIntegrationsPermission
 from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.fields.empty_integer import EmptyIntegerField
@@ -28,6 +29,8 @@ class RepositorySerializer(serializers.Serializer):
             ("active", "active"),
         )
     )
+    name = serializers.CharField(required=False)
+    url = serializers.URLField(required=False, allow_blank=True)
     integrationId = EmptyIntegerField(required=False, allow_null=True)
 
 
@@ -69,6 +72,15 @@ class OrganizationRepositoryDetailsEndpoint(OrganizationEndpoint):
             update_kwargs["integration_id"] = integration.id
             update_kwargs["provider"] = f"integrations:{integration.provider}"
 
+        if (
+            features.has("organizations:integrations-custom-scm", organization, actor=request.user)
+            and repo.provider == "integrations:custom_scm"
+        ):
+            if result.get("name"):
+                update_kwargs["name"] = result["name"]
+            if result.get("url") is not None:
+                update_kwargs["url"] = result["url"] or None
+
         if update_kwargs:
             old_status = repo.status
             with transaction.atomic():

+ 20 - 1
src/sentry/integrations/custom_scm/integration.py

@@ -52,6 +52,19 @@ class CustomSCMIntegration(IntegrationInstallation, RepositoryMixin):
     def get_client(self):
         pass
 
+    def get_stacktrace_link(self, repo, filepath, default, version):
+        """
+        We don't have access to verify that the file does exists
+        (using `check_file`) so instead we just return the
+        formatted source url using the default branch provided.
+        """
+        return self.format_source_url(repo, filepath, default)
+
+    def format_source_url(self, repo, filepath, branch):
+        # This format works for GitHub/GitLab, not sure if it would
+        # need to change for a different provider
+        return f"{repo.url}/blob/{branch}/{filepath}"
+
     def get_repositories(self, query=None):
         """
         Used to get any repositories that are not already tied
@@ -112,7 +125,13 @@ class CustomSCMIntegrationProvider(IntegrationProvider):
     requires_feature_flag = True
     metadata = metadata
     integration_cls = CustomSCMIntegration
-    features = frozenset([IntegrationFeatures.COMMITS, IntegrationFeatures.STACKTRACE_LINK])
+    features = frozenset(
+        [
+            IntegrationFeatures.COMMITS,
+            IntegrationFeatures.STACKTRACE_LINK,
+            IntegrationFeatures.CODEOWNERS,
+        ]
+    )
 
     def get_pipeline_views(self):
         return [InstallationConfigView()]

+ 34 - 5
src/sentry/integrations/custom_scm/repository.py

@@ -1,7 +1,10 @@
 import logging
 
+from django.http import Http404
 from rest_framework.response import Response
 
+from sentry.api.serializers import serialize
+from sentry.models import Integration, Repository
 from sentry.plugins import providers
 
 
@@ -13,8 +16,34 @@ class CustomSCMRepositoryProvider(providers.IntegrationRepositoryProvider):
         return repo.name
 
     def dispatch(self, request, organization, **kwargs):
-        # TODO(meredith): Add functionality to actually add a repo
-        # to the manual integration.
-        #   * update provider
-        #   * update integration_id
-        return Response(status=200)
+        """
+        Adding a repository to the Custom SCM integration is
+        just two steps:
+           1. Change the provider from `null` to 'integrations:custom_scm'
+           2. Add the integration_id that is passed from the request
+
+        We set the `identifier` to be the repo's id in our db
+        when we call `get_repositories`. Normally this is the id or
+        identifier in the other service (i.e. the GH repo id)
+        """
+        repo_id = request.data.get("identifier")
+        integration_id = request.data.get("installation")
+
+        try:
+            # double check the repository_id passed is not
+            # for an already 'claimed' repository
+            repo = Repository.objects.get(
+                organization_id=organization.id,
+                id=repo_id,
+                integration_id__isnull=True,
+                provider__isnull=True,
+            )
+            integration = Integration.objects.get(organizations=organization, id=integration_id)
+        except (Repository.DoesNotExist, Integration.DoesNotExist):
+            raise Http404
+
+        repo.provider = self.id
+        repo.integration_id = integration.id
+        repo.save()
+
+        return Response(serialize(repo, request.user), status=201)

+ 85 - 0
static/app/components/repositoryEditForm.tsx

@@ -0,0 +1,85 @@
+import React from 'react';
+
+import {IconWarning} from 'app/icons';
+import {t, tct} from 'app/locale';
+import {Repository} from 'app/types';
+import {FieldFromConfig} from 'app/views/settings/components/forms';
+import Form from 'app/views/settings/components/forms/form';
+import {Field} from 'app/views/settings/components/forms/type';
+
+import Alert from './alert';
+
+type Props = Pick<Form['props'], 'onSubmitSuccess' | 'onCancel'> & {
+  orgSlug: string;
+  repository: Repository;
+  onSubmitSuccess: (data: any) => void;
+  closeModal: () => void;
+};
+
+export default class RepositoryEditForm extends React.Component<Props> {
+  get initialData() {
+    const {repository} = this.props;
+
+    return {
+      name: repository.name,
+      url: repository.url || '',
+    };
+  }
+
+  get formFields(): Field[] {
+    const fields: any[] = [
+      {
+        name: 'name',
+        type: 'string',
+        required: true,
+        label: t('Name of your repository.'),
+      },
+      {
+        name: 'url',
+        type: 'string',
+        required: false,
+        label: t('Full URL to your repository.'),
+        placeholder: t('https://github.com/my-org/my-repo/'),
+      },
+    ];
+    return fields;
+  }
+
+  render() {
+    const {onCancel, orgSlug, repository} = this.props;
+    const endpoint = `/organizations/${orgSlug}/repos/${repository.id}/`;
+    return (
+      <Form
+        initialData={this.initialData}
+        onSubmitSuccess={data => {
+          this.props.onSubmitSuccess(data);
+          this.props.closeModal();
+        }}
+        apiEndpoint={endpoint}
+        apiMethod="PUT"
+        onCancel={onCancel}
+      >
+        <Alert type="warning" icon={<IconWarning />}>
+          {tct(
+            'Changing the [name:repo name] may have consequences if it no longer matches the repo name used when [link:sending commits with releases].',
+            {
+              link: (
+                <a href="https://docs.sentry.io/product/cli/releases/#sentry-cli-commit-integration" />
+              ),
+              name: <strong>repo name</strong>,
+            }
+          )}
+        </Alert>
+        {this.formFields.map(field => (
+          <FieldFromConfig
+            key={field.name}
+            field={field}
+            inline={false}
+            stacked
+            flexibleControlStateSize
+          />
+        ))}
+      </Form>
+    );
+  }
+}

+ 87 - 28
static/app/components/repositoryRow.tsx

@@ -1,23 +1,27 @@
-import {Component} from 'react';
+import {Component, Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import {cancelDeleteRepository, deleteRepository} from 'app/actionCreators/integrations';
+import {openModal} from 'app/actionCreators/modal';
 import {Client} from 'app/api';
 import Access from 'app/components/acl/access';
 import Button from 'app/components/button';
 import Confirm from 'app/components/confirm';
 import {PanelItem} from 'app/components/panels';
+import RepositoryEditForm from 'app/components/repositoryEditForm';
 import Tooltip from 'app/components/tooltip';
-import {IconDelete} from 'app/icons';
+import {IconDelete, IconEdit} from 'app/icons';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
-import {Repository, RepositoryStatus} from 'app/types';
+import {Organization, Repository, RepositoryStatus} from 'app/types';
+import withOrganization from 'app/utils/withOrganization';
 
 type DefaultProps = {
   showProvider?: boolean;
 };
 
 type Props = DefaultProps & {
+  organization: Organization;
   repository: Repository;
   api: Client;
   orgId: string;
@@ -68,13 +72,71 @@ class RepositoryRow extends Component<Props> {
     );
   };
 
+  handleEditRepo = (data: Repository) => {
+    const {onRepositoryChange} = this.props;
+    if (onRepositoryChange) {
+      onRepositoryChange(data);
+    }
+  };
+
   get isActive() {
     return this.props.repository.status === RepositoryStatus.ACTIVE;
   }
 
+  renderDeleteButton(hasAccess) {
+    const {repository} = this.props;
+    const isActive = this.isActive;
+    return (
+      <Tooltip
+        title={t(
+          'You must be an organization owner, manager or admin to remove a repository.'
+        )}
+        disabled={hasAccess}
+      >
+        <Confirm
+          disabled={
+            !hasAccess || (!isActive && repository.status !== RepositoryStatus.DISABLED)
+          }
+          onConfirm={this.deleteRepo}
+          message={t(
+            'Are you sure you want to remove this repository? All associated commit data will be removed in addition to the repository.'
+          )}
+        >
+          <StyledButton
+            size="xsmall"
+            icon={<IconDelete size="xs" />}
+            label={t('delete')}
+            disabled={!hasAccess}
+          />
+        </Confirm>
+      </Tooltip>
+    );
+  }
+
+  openModal = () => {
+    const {repository, orgId} = this.props;
+    openModal(({Body, Header, closeModal}) => (
+      <Fragment>
+        <Header closeButton>{t('Edit Repository')}</Header>
+        <Body>
+          <RepositoryEditForm
+            orgSlug={orgId}
+            repository={repository}
+            onSubmitSuccess={this.handleEditRepo}
+            closeModal={closeModal}
+            onCancel={closeModal}
+          />
+        </Body>
+      </Fragment>
+    ));
+  };
+
   render() {
-    const {repository, showProvider} = this.props;
+    const {repository, showProvider, organization} = this.props;
     const isActive = this.isActive;
+    const isCustomRepo =
+      organization.features.includes('integrations-custom-scm') &&
+      repository.provider.id === 'integrations:custom_scm';
 
     return (
       <Access access={['org:integrations']}>
@@ -105,31 +167,23 @@ class RepositoryRow extends Component<Props> {
                 )}
               </div>
             </RepositoryTitleAndUrl>
-
-            <Tooltip
-              title={t(
-                'You must be an organization owner, manager or admin to remove a repository.'
-              )}
-              disabled={hasAccess}
-            >
-              <Confirm
-                disabled={
-                  !hasAccess ||
-                  (!isActive && repository.status !== RepositoryStatus.DISABLED)
-                }
-                onConfirm={this.deleteRepo}
-                message={t(
-                  'Are you sure you want to remove this repository? All associated commit data will be removed in addition to the repository.'
-                )}
-              >
-                <Button
+            {isCustomRepo ? (
+              <EditAndDelete>
+                <StyledButton
                   size="xsmall"
-                  icon={<IconDelete size="xs" />}
-                  label={t('delete')}
-                  disabled={!hasAccess}
+                  icon={<IconEdit size="xs" />}
+                  label={t('edit')}
+                  disabled={
+                    !hasAccess ||
+                    (!isActive && repository.status !== RepositoryStatus.DISABLED)
+                  }
+                  onClick={() => this.openModal()}
                 />
-              </Confirm>
-            </Tooltip>
+                {this.renderDeleteButton(hasAccess)}
+              </EditAndDelete>
+            ) : (
+              this.renderDeleteButton(hasAccess)
+            )}
           </StyledPanelItem>
         )}
       </Access>
@@ -165,10 +219,15 @@ const RepositoryTitleAndUrl = styled('div')`
   flex-direction: column;
 `;
 
+const EditAndDelete = styled('div')`
+  display: flex;
+  margin-left: ${space(1)};
+`;
+
 const RepositoryTitle = styled('div')`
   margin-bottom: ${space(1)};
   /* accommodate cancel button height */
   line-height: 26px;
 `;
 
-export default RepositoryRow;
+export default withOrganization(RepositoryRow);

+ 4 - 0
static/app/views/organizationIntegrations/integrationRepos.tsx

@@ -68,6 +68,10 @@ class IntegrationRepos extends AsyncComponent<Props, State> {
     itemList.forEach(item => {
       if (item.id === data.id) {
         item.status = data.status;
+        // allow for custom scm repositories to be updated, and
+        // url is optional and therefore can be an empty string
+        item.url = data.url === undefined ? item.url : data.url;
+        item.name = data.name || item.name;
       }
     });
     this.setState({itemList});

+ 99 - 6
tests/js/spec/components/repositoryRow.spec.jsx

@@ -13,6 +13,17 @@ describe('RepositoryRow', function () {
   const pendingRepo = TestStubs.Repository({
     status: 'pending_deletion',
   });
+  const customRepo = TestStubs.Repository({
+    provider: {
+      id: 'integrations:custom_scm',
+    },
+  });
+  const customPendingRepo = TestStubs.Repository({
+    provider: {
+      id: 'integrations:custom_scm',
+    },
+    status: 'pending_deletion',
+  });
   const api = new Client();
 
   describe('rendering with access', function () {
@@ -23,7 +34,12 @@ describe('RepositoryRow', function () {
 
     it('displays provider information', function () {
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={repository} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={repository}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
       expect(wrapper.find('strong').text()).toEqual(repository.name);
@@ -38,7 +54,12 @@ describe('RepositoryRow', function () {
 
     it('displays cancel pending button', function () {
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={pendingRepo} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={pendingRepo}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
 
@@ -61,7 +82,12 @@ describe('RepositoryRow', function () {
 
     it('displays disabled trash', function () {
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={repository} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={repository}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
 
@@ -72,7 +98,12 @@ describe('RepositoryRow', function () {
 
     it('displays disabled cancel', function () {
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={pendingRepo} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={pendingRepo}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
 
@@ -98,7 +129,12 @@ describe('RepositoryRow', function () {
       });
 
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={repository} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={repository}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
       wrapper.find('Button[label="delete"]').simulate('click');
@@ -127,7 +163,12 @@ describe('RepositoryRow', function () {
       });
 
       const wrapper = mountWithTheme(
-        <RepositoryRow repository={pendingRepo} api={api} orgId={organization.slug} />,
+        <RepositoryRow
+          repository={pendingRepo}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
         routerContext
       );
       wrapper.find('Button[data-test-id="repo-cancel"]').simulate('click');
@@ -136,4 +177,56 @@ describe('RepositoryRow', function () {
       expect(cancel).toHaveBeenCalled();
     });
   });
+
+  describe('renders custom_scm repo', function () {
+    const organization = TestStubs.Organization({
+      access: ['org:integrations'],
+      features: ['integrations-custom-scm'],
+    });
+    const routerContext = TestStubs.routerContext([{organization}]);
+
+    it('displays edit button', function () {
+      const wrapper = mountWithTheme(
+        <RepositoryRow
+          repository={customRepo}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
+        routerContext
+      );
+
+      // Trash button should display enabled
+      expect(wrapper.find('Confirm').props().disabled).toEqual(false);
+      // No cancel button
+      expect(wrapper.find('Button[data-test-id="repo-cancel"]')).toHaveLength(0);
+
+      // Edit button should display enabled
+      expect(wrapper.find('Button[label="edit"]').props().disabled).toEqual(false);
+    });
+
+    it('disables edit button when cancel pending', function () {
+      const wrapper = mountWithTheme(
+        <RepositoryRow
+          repository={customPendingRepo}
+          api={api}
+          orgId={organization.slug}
+          organization={organization}
+        />,
+        routerContext
+      );
+
+      // Trash button should be disabled
+      expect(wrapper.find('Confirm').props().disabled).toEqual(true);
+      expect(wrapper.find('Button[label="delete"]').props().disabled).toEqual(true);
+
+      // Edit button should be disabled
+      expect(wrapper.find('Button[label="edit"]').props().disabled).toEqual(true);
+
+      // Cancel button active
+      const cancel = wrapper.find('Button[data-test-id="repo-cancel"]');
+      expect(cancel).toHaveLength(1);
+      expect(cancel.props().disabled).toEqual(false);
+    });
+  });
 });

+ 67 - 0
tests/sentry/api/endpoints/test_organization_repository_details.py

@@ -3,6 +3,7 @@ from django.urls import reverse
 from sentry.constants import ObjectStatus
 from sentry.models import Commit, Integration, OrganizationOption, Repository
 from sentry.testutils import APITestCase
+from sentry.testutils.helpers import with_feature
 from sentry.utils.compat.mock import patch
 
 
@@ -156,6 +157,72 @@ class OrganizationRepositoryDeleteTest(APITestCase):
         assert repo.status == ObjectStatus.VISIBLE
         assert repo.integration_id == integration.id
 
+    @with_feature("organizations:integrations-custom-scm")
+    def test_put_custom_scm_repo(self):
+        """
+        Allow repositories that are tied to Custom SCM integrations
+        to be able to update `name` and `url`.
+        """
+        self.login_as(user=self.user)
+
+        org = self.create_organization(owner=self.user, name="baz")
+        integration = Integration.objects.create(
+            provider="integrations:custom_scm", name="some-org"
+        )
+        integration.add_organization(org)
+
+        repo = Repository.objects.create(
+            name="some-org/example",
+            organization_id=org.id,
+            integration_id=integration.id,
+            provider="integrations:custom_scm",
+        )
+
+        url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
+        response = self.client.put(
+            url, data={"url": "https://example.com/some-org/repo", "name": "some-org/new-name"}
+        )
+
+        assert response.status_code == 200
+        repo = Repository.objects.get(id=repo.id)
+        assert repo.url == "https://example.com/some-org/repo"
+        assert repo.name == "some-org/new-name"
+
+        # test that empty url sets it back to None
+        response = self.client.put(url, data={"url": ""})
+        repo = Repository.objects.get(id=repo.id)
+        assert repo.url is None
+        assert repo.name == "some-org/new-name"
+
+    @with_feature("organizations:integrations-custom-scm")
+    def test_no_name_or_url_updates(self):
+        """
+        Repositories that are not tied to Custom SCM integrations
+        *cannot* update their `name` or `url`.
+
+        This is true for repos in the following categories:
+         * 'Unknown provider'
+         * Plugins
+         * First Party Integrations (non Custom SCM)
+        """
+        self.login_as(user=self.user)
+
+        org = self.create_organization(owner=self.user, name="baz")
+        repo = Repository.objects.create(
+            name="example", organization_id=org.id, url="https:/example.com/"
+        )
+
+        url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
+        response = self.client.put(
+            url, data={"url": "https://example.com/some-org/repo", "name": "some-org/new-name"}
+        )
+
+        assert response.status_code == 200
+        repo = Repository.objects.get(id=repo.id)
+        # no changes were made
+        assert repo.url == "https:/example.com/"
+        assert repo.name == "example"
+
     def test_put_cancel_deletion(self):
         self.login_as(user=self.user)
 

+ 78 - 0
tests/sentry/integrations/custom_scm/test_repository.py

@@ -0,0 +1,78 @@
+from uuid import uuid4
+
+from exam import fixture
+
+from sentry.integrations.custom_scm.repository import CustomSCMRepositoryProvider
+from sentry.models import Integration, Repository
+from sentry.testutils import IntegrationRepositoryTestCase
+
+
+class CustomSCMRepositoryProviderTest(IntegrationRepositoryTestCase):
+    provider_name = "integrations:custom_scm"
+
+    def setUp(self):
+        super().setUp()
+        self.external_id = uuid4().hex
+        self.integration = Integration.objects.create(
+            provider="custom_scm",
+            name="Example Custom SCM",
+            external_id=self.external_id,
+            metadata={
+                "domain_name": "http://example.com/some-org/",
+            },
+        )
+        self.integration.add_organization(self.organization, self.user)
+        self.integration.get_provider().setup()
+
+        self.repo = Repository.objects.create(
+            name="some-repo", organization_id=self.organization.id
+        )
+        self.repository_data = {
+            "provider": None,
+            "installation": self.integration.id,
+            "id": self.repo.id,
+        }
+
+    @fixture
+    def provider(self):
+        return CustomSCMRepositoryProvider("custom_scm")
+
+    def _get_repo_data(self, repo):
+        return {
+            "provider": None,
+            "installation": self.integration.id,
+            "id": repo.id,
+        }
+
+    def test_create_repository(self):
+        response = self.create_repository(
+            self._get_repo_data(self.repo), self.integration.id, add_responses=False
+        )
+        assert response.status_code == 201
+
+        repo = Repository.objects.get(id=self.repo.id)
+        assert repo.integration_id == self.integration.id
+        assert repo.provider == "integrations:custom_scm"
+
+    def test_non_null_provider(self):
+        # no integration_id, but has provider
+        repo = Repository.objects.create(
+            name="new-repo", organization_id=self.organization.id, provider="github"
+        )
+        response = self.create_repository(
+            self._get_repo_data(repo), self.integration.id, add_responses=False
+        )
+        assert response.status_code == 404
+
+    def test_non_null_integration_id(self):
+        # has both integration_id and provider
+        repo = Repository.objects.create(
+            name="new-repo",
+            organization_id=self.organization.id,
+            provider="integrations:custom_scm",
+            integration_id=self.integration.id,
+        )
+        response = self.create_repository(
+            self._get_repo_data(repo), self.integration.id, add_responses=False
+        )
+        assert response.status_code == 404