Browse Source

types(python): Azure DevOps Integration (#28481)

Marcos Gaeta 3 years ago
parent
commit
85122e2463

+ 3 - 0
mypy.ini

@@ -35,6 +35,7 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/integrations/slack/unfurl/*.py,
         src/sentry/integrations/slack/util/*.py,
         src/sentry/integrations/slack/views/*.py,
+        src/sentry/integrations/vsts/**/*.py,
         src/sentry/killswitches.py,
         src/sentry/lang/native/appconnect.py,
         src/sentry/models/debugfile.py,
@@ -89,6 +90,8 @@ ignore_missing_imports = True
 ignore_missing_imports = True
 [mypy-jsonschema]
 ignore_missing_imports = True
+[mypy-mistune.*]
+ignore_missing_imports = True
 [mypy-rb.*]
 ignore_missing_imports = True
 [mypy-rest_framework.*]

+ 2 - 1
src/sentry/integrations/vsts/__init__.py

@@ -3,6 +3,7 @@ from sentry.utils.imports import import_submodules
 
 from .notify_action import AzureDevopsCreateTicketAction
 
-import_submodules(globals(), __name__, __path__)
+path = __path__  # type: ignore
+import_submodules(globals(), __name__, path)
 
 rules.add(AzureDevopsCreateTicketAction)

+ 68 - 44
src/sentry/integrations/vsts/client.py

@@ -1,8 +1,17 @@
+from typing import TYPE_CHECKING, Any, List, Mapping, Optional, Sequence, Union
+
+from rest_framework.response import Response
+
 from sentry.integrations.client import ApiClient, OAuth2RefreshMixin
 from sentry.utils.http import absolute_uri
 
+if TYPE_CHECKING:
+    from sentry.models import Identity, Project
+
 UNSET = object()
 
+UnsettableString = Union[str, object, None]
+
 FIELD_MAP = {
     "title": "/fields/System.Title",
     "description": "/fields/System.Description",
@@ -37,19 +46,29 @@ class VstsApiPath:
     work_item_categories = "{instance}{project}/_apis/wit/workitemtypecategories"
 
 
-class VstsApiClient(ApiClient, OAuth2RefreshMixin):
+class VstsApiClient(ApiClient, OAuth2RefreshMixin):  # type: ignore
     api_version = "4.1"  # TODO: update api version
     api_version_preview = "-preview.1"
     integration_name = "vsts"
 
-    def __init__(self, identity, oauth_redirect_url, *args, **kwargs):
+    def __init__(
+        self, identity: "Identity", oauth_redirect_url: str, *args: Any, **kwargs: Any
+    ) -> None:
         super().__init__(*args, **kwargs)
         self.identity = identity
         self.oauth_redirect_url = oauth_redirect_url
         if "access_token" not in self.identity.data:
             raise ValueError("Vsts Identity missing access token")
 
-    def request(self, method, path, data=None, params=None, api_preview=False, timeout=None):
+    def request(
+        self,
+        method: str,
+        path: str,
+        data: Optional[Mapping[str, Any]] = None,
+        params: Optional[Sequence[Any]] = None,
+        api_preview: bool = False,
+        timeout: Optional[int] = None,
+    ) -> Response:
         self.check_auth(redirect_url=self.oauth_redirect_url)
         headers = {
             "Accept": "application/json; api-version={}{}".format(
@@ -68,14 +87,14 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
 
     def create_work_item(
         self,
-        instance,
-        project,
-        item_type=None,
-        title=None,
-        description=None,
-        comment=None,
-        link=None,
-    ):
+        instance: str,
+        project: "Project",
+        item_type: Optional[str] = None,
+        title: Optional[str] = None,
+        description: Optional[str] = None,
+        comment: Optional[str] = None,
+        link: Optional[str] = None,
+    ) -> Response:
         data = []
         if title:
             data.append({"op": "add", "path": FIELD_MAP["title"], "value": title})
@@ -103,16 +122,16 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
 
     def update_work_item(
         self,
-        instance,
-        id,
-        title=UNSET,
-        description=UNSET,
-        link=UNSET,
-        comment=UNSET,
-        assigned_to=UNSET,
-        state=UNSET,
-    ):
-        data = []
+        instance: str,
+        id: str,
+        title: UnsettableString = UNSET,
+        description: UnsettableString = UNSET,
+        link: UnsettableString = UNSET,
+        comment: UnsettableString = UNSET,
+        assigned_to: UnsettableString = UNSET,
+        state: UnsettableString = UNSET,
+    ) -> Response:
+        data: List[Mapping[str, Any]] = []
 
         for f_name, f_value in (
             ("title", title),
@@ -143,10 +162,10 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
 
         return self.patch(VstsApiPath.work_items.format(instance=instance, id=id), data=data)
 
-    def get_work_item(self, instance, id):
+    def get_work_item(self, instance: str, id: str) -> Response:
         return self.get(VstsApiPath.work_items.format(instance=instance, id=id))
 
-    def get_work_item_states(self, instance, project):
+    def get_work_item_states(self, instance: str, project: str) -> Response:
         return self.get(
             VstsApiPath.work_item_states.format(
                 instance=instance,
@@ -157,10 +176,10 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             api_preview=True,
         )
 
-    def get_work_item_categories(self, instance, project):
+    def get_work_item_categories(self, instance: str, project: str) -> Response:
         return self.get(VstsApiPath.work_item_categories.format(instance=instance, project=project))
 
-    def get_repo(self, instance, name_or_id, project=None):
+    def get_repo(self, instance: str, name_or_id: str, project: Optional[str] = None) -> Response:
         return self.get(
             VstsApiPath.repository.format(
                 instance=instance,
@@ -169,7 +188,7 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             )
         )
 
-    def get_repos(self, instance, project=None):
+    def get_repos(self, instance: str, project: Optional[str] = None) -> Response:
         return self.get(
             VstsApiPath.repositories.format(
                 instance=instance, project=f"{project}/" if project else ""
@@ -177,25 +196,27 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             timeout=5,
         )
 
-    def get_commits(self, instance, repo_id, commit, limit=100):
+    def get_commits(self, instance: str, repo_id: str, commit: str, limit: int = 100) -> Response:
         return self.get(
             VstsApiPath.commits.format(instance=instance, repo_id=repo_id),
             params={"commit": commit, "$top": limit},
         )
 
-    def get_commit(self, instance, repo_id, commit):
+    def get_commit(self, instance: str, repo_id: str, commit: str) -> Response:
         return self.get(
             VstsApiPath.commit.format(instance=instance, repo_id=repo_id, commit_id=commit)
         )
 
-    def get_commit_filechanges(self, instance, repo_id, commit):
+    def get_commit_filechanges(self, instance: str, repo_id: str, commit: str) -> Response:
         resp = self.get(
             VstsApiPath.commits_changes.format(instance=instance, repo_id=repo_id, commit_id=commit)
         )
         changes = resp["changes"]
         return changes
 
-    def get_commit_range(self, instance, repo_id, start_sha, end_sha):
+    def get_commit_range(
+        self, instance: str, repo_id: str, start_sha: str, end_sha: str
+    ) -> Response:
         return self.post(
             VstsApiPath.commits_batch.format(instance=instance, repo_id=repo_id),
             data={
@@ -204,22 +225,25 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             },
         )
 
-    def get_project(self, instance, project_id):
+    def get_project(self, instance: str, project_id: str) -> Response:
         return self.get(
             VstsApiPath.project.format(instance=instance, project_id=project_id),
             params={"stateFilter": "WellFormed"},
         )
 
-    def get_projects(self, instance):
-        def gen_params(page_number, page_size):
-            # ADO supports a continuation token in the response but only in the newer API version (https://docs.microsoft.com/en-us/rest/api/azure/devops/core/projects/list?view=azure-devops-rest-6.1)
-            # the token comes as a response header instead of the body and our API clients currently only return the body
-            # we can use count, $skip, and $top to get the same result
+    def get_projects(self, instance: str) -> Response:
+        def gen_params(page_number: int, page_size: int) -> Mapping[str, Union[str, int]]:
+            # ADO supports a continuation token in the response but only in the newer API version (
+            # https://docs.microsoft.com/en-us/rest/api/azure/devops/core/projects/list?view=azure-devops-rest-6.1
+            # ). The token comes as a response header instead of the body and our API clients
+            # currently only return the body we can use count, $skip, and $top to get the same result.
             offset = self.page_size * page_number
             return {"stateFilter": "WellFormed", "$skip": offset, "$top": page_size}
 
-        def get_results(resp):
-            return resp["value"]
+        def get_results(resp: Response) -> Sequence[Any]:
+            # Explicitly typing to satisfy mypy.
+            results: Sequence[Any] = resp["value"]
+            return results
 
         return self.get_with_pagination(
             VstsApiPath.projects.format(instance=instance),
@@ -227,7 +251,7 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             get_results=get_results,
         )
 
-    def get_users(self, account_name, continuation_token=None):
+    def get_users(self, account_name: str, continuation_token: Optional[str] = None) -> Response:
         """
         Gets Users with access to a given account/organization
         https://docs.microsoft.com/en-us/rest/api/azure/devops/graph/users/list?view=azure-devops-rest-4.1
@@ -238,7 +262,7 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             params={"continuationToken": continuation_token},
         )
 
-    def create_subscription(self, instance, shared_secret):
+    def create_subscription(self, instance: Optional[str], shared_secret: str) -> Response:
         return self.post(
             VstsApiPath.subscriptions.format(instance=instance),
             data={
@@ -255,22 +279,22 @@ class VstsApiClient(ApiClient, OAuth2RefreshMixin):
             },
         )
 
-    def get_subscription(self, instance, subscription_id):
+    def get_subscription(self, instance: str, subscription_id: str) -> Response:
         return self.get(
             VstsApiPath.subscription.format(instance=instance, subscription_id=subscription_id)
         )
 
-    def delete_subscription(self, instance, subscription_id):
+    def delete_subscription(self, instance: str, subscription_id: str) -> Response:
         return self.delete(
             VstsApiPath.subscription.format(instance=instance, subscription_id=subscription_id)
         )
 
-    def update_subscription(self, instance, subscription_id):
+    def update_subscription(self, instance: str, subscription_id: str) -> Response:
         return self.put(
             VstsApiPath.subscription.format(instance=instance, subscription_id=subscription_id)
         )
 
-    def search_issues(self, account_name, query=None):
+    def search_issues(self, account_name: str, query: Optional[str] = None) -> Response:
         return self.post(
             VstsApiPath.work_item_search.format(account_name=account_name),
             data={"searchText": query, "$top": 1000},

+ 64 - 43
src/sentry/integrations/vsts/integration.py

@@ -1,9 +1,13 @@
 import logging
 import re
 from time import time
+from typing import Any, Mapping, MutableMapping, Optional, Sequence, Tuple
 
 from django import forms
+from django.db.models import QuerySet
 from django.utils.translation import ugettext as _
+from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry import features, http
 from sentry.auth.exceptions import IdentityNotValid
@@ -19,6 +23,7 @@ from sentry.integrations import (
 )
 from sentry.integrations.repositories import RepositoryMixin
 from sentry.integrations.vsts.issues import VstsIssueSync
+from sentry.models import Identity
 from sentry.models import Integration as IntegrationModel
 from sentry.models import (
     IntegrationExternalProject,
@@ -26,7 +31,7 @@ from sentry.models import (
     OrganizationIntegration,
     Repository,
 )
-from sentry.pipeline import NestedPipelineView, PipelineView
+from sentry.pipeline import NestedPipelineView, Pipeline, PipelineView
 from sentry.shared_integrations.exceptions import (
     ApiError,
     IntegrationError,
@@ -34,6 +39,7 @@ from sentry.shared_integrations.exceptions import (
 )
 from sentry.tasks.integrations import migrate_repo
 from sentry.utils.http import absolute_uri
+from sentry.utils.json import JSONData
 from sentry.web.helpers import render_to_response
 
 from .client import VstsApiClient
@@ -99,7 +105,7 @@ metadata = IntegrationMetadata(
 logger = logging.getLogger("sentry.integrations")
 
 
-class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
+class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):  # type: ignore
     logger = logger
     comment_key = "sync_comments"
     outbound_status_key = "sync_status_forward"
@@ -107,17 +113,17 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
     outbound_assignee_key = "sync_forward_assignment"
     inbound_assignee_key = "sync_reverse_assignment"
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
-        self.default_identity = None
+        self.default_identity: Optional[Identity] = None
 
-    def reinstall(self):
+    def reinstall(self) -> None:
         self.reinstall_repositories()
 
-    def all_repos_migrated(self):
+    def all_repos_migrated(self) -> bool:
         return not self.get_unmigratable_repositories()
 
-    def get_repositories(self, query=None):
+    def get_repositories(self, query: Optional[str] = None) -> Sequence[Mapping[str, str]]:
         try:
             repos = self.get_client().get_repos(self.instance)
         except (ApiError, IdentityNotValid) as e:
@@ -132,12 +138,12 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
             )
         return data
 
-    def get_unmigratable_repositories(self):
+    def get_unmigratable_repositories(self) -> QuerySet:
         return Repository.objects.filter(
             organization_id=self.organization_id, provider="visualstudio"
         ).exclude(external_id__in=[r["identifier"] for r in self.get_repositories()])
 
-    def has_repo_access(self, repo):
+    def has_repo_access(self, repo: Repository) -> bool:
         client = self.get_client()
         try:
             # since we don't actually use webhooks for vsts commits,
@@ -147,32 +153,31 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
             return False
         return True
 
-    def get_client(self):
+    def get_client(self) -> VstsApiClient:
         if self.default_identity is None:
             self.default_identity = self.get_default_identity()
 
-        self.check_domain_name()
+        self.check_domain_name(self.default_identity)
         return VstsApiClient(self.default_identity, VstsIntegrationProvider.oauth_redirect_url)
 
-    def check_domain_name(self):
+    def check_domain_name(self, default_identity: Identity) -> None:
         if re.match("^https://.+/$", self.model.metadata["domain_name"]):
             return
+
         base_url = VstsIntegrationProvider.get_base_url(
-            self.default_identity.data["access_token"], self.model.external_id
+            default_identity.data["access_token"], self.model.external_id
         )
         self.model.metadata["domain_name"] = base_url
         self.model.save()
 
-    def get_organization_config(self):
+    def get_organization_config(self) -> Sequence[Mapping[str, Any]]:
         client = self.get_client()
         instance = self.model.metadata["domain_name"]
 
         project_selector = []
-
+        all_states_set = set()
         try:
             projects = client.get_projects(instance)
-            all_states = set()
-
             for idx, project in enumerate(projects):
                 project_selector.append({"value": project["id"], "label": project["name"]})
                 # only request states for the first 5 projects to limit number
@@ -180,9 +185,9 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
                 if idx <= 5:
                     project_states = client.get_work_item_states(instance, project["id"])["value"]
                     for state in project_states:
-                        all_states.add(state["name"])
+                        all_states_set.add(state["name"])
 
-            all_states = [(state, state) for state in all_states]
+            all_states = [(state, state) for state in all_states_set]
             disabled = False
         except (ApiError, IdentityNotValid):
             all_states = []
@@ -258,7 +263,7 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
 
         return fields
 
-    def update_organization_config(self, data):
+    def update_organization_config(self, data: MutableMapping[str, Any]) -> None:
         if "sync_status_forward" in data:
             project_ids_and_statuses = data.pop("sync_status_forward")
             if any(
@@ -285,8 +290,8 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
         config.update(data)
         self.org_integration.update(config=config)
 
-    def get_config_data(self):
-        config = self.org_integration.config
+    def get_config_data(self) -> Mapping[str, Any]:
+        config: MutableMapping[str, Any] = self.org_integration.config
         project_mappings = IntegrationExternalProject.objects.filter(
             organization_integration_id=self.org_integration.id
         )
@@ -300,18 +305,22 @@ class VstsIntegration(IntegrationInstallation, RepositoryMixin, VstsIssueSync):
         return config
 
     @property
-    def instance(self):
-        return self.model.metadata["domain_name"]
+    def instance(self) -> str:
+        # Explicitly typing to satisfy mypy.
+        instance_: str = self.model.metadata["domain_name"]
+        return instance_
 
     @property
-    def default_project(self):
+    def default_project(self) -> Optional[str]:
         try:
-            return self.model.metadata["default_project"]
+            # Explicitly typing to satisfy mypy.
+            default_project_: str = self.model.metadata["default_project"]
         except KeyError:
             return None
+        return default_project_
 
 
-class VstsIntegrationProvider(IntegrationProvider):
+class VstsIntegrationProvider(IntegrationProvider):  # type: ignore
     key = "vsts"
     name = "Azure DevOps"
     metadata = metadata
@@ -333,7 +342,12 @@ class VstsIntegrationProvider(IntegrationProvider):
 
     VSTS_ACCOUNT_LOOKUP_URL = "https://app.vssps.visualstudio.com/_apis/resourceareas/79134C72-4A58-4B42-976C-04E7115F32BF?hostId=%s&api-preview=5.0-preview.1"
 
-    def post_install(self, integration, organization, extra=None):
+    def post_install(
+        self,
+        integration: IntegrationModel,
+        organization: Organization,
+        extra: Optional[Mapping[str, Any]] = None,
+    ) -> None:
         repo_ids = Repository.objects.filter(
             organization_id=organization.id,
             provider__in=["visualstudio", "integrations:vsts"],
@@ -349,13 +363,13 @@ class VstsIntegrationProvider(IntegrationProvider):
                 }
             )
 
-    def get_scopes(self):
+    def get_scopes(self) -> Sequence[str]:
         if use_limited_scopes(self.pipeline):
             return ("vso.graph", "vso.serviceendpoint_manage", "vso.work_write")
 
         return ("vso.code", "vso.graph", "vso.serviceendpoint_manage", "vso.work_write")
 
-    def get_pipeline_views(self):
+    def get_pipeline_views(self) -> Sequence[PipelineView]:
         identity_pipeline_config = {
             "redirect_url": absolute_uri(self.oauth_redirect_url),
             "oauth_scopes": self.get_scopes(),
@@ -370,7 +384,7 @@ class VstsIntegrationProvider(IntegrationProvider):
 
         return [identity_pipeline_view, AccountConfigView()]
 
-    def build_integration(self, state):
+    def build_integration(self, state: Mapping[str, Any]) -> Mapping[str, Any]:
         data = state["identity"]["data"]
         oauth_data = self.get_oauth_data(data)
         account = state["account"]
@@ -378,7 +392,7 @@ class VstsIntegrationProvider(IntegrationProvider):
         scopes = sorted(self.get_scopes())
         base_url = self.get_base_url(data["access_token"], account["accountId"])
 
-        integration = {
+        integration: MutableMapping[str, Any] = {
             "name": account["accountName"],
             "external_id": account["accountId"],
             "metadata": {"domain_name": base_url, "scopes": scopes},
@@ -412,7 +426,9 @@ class VstsIntegrationProvider(IntegrationProvider):
 
         return integration
 
-    def create_subscription(self, instance, oauth_data):
+    def create_subscription(
+        self, instance: Optional[str], oauth_data: Mapping[str, Any]
+    ) -> Tuple[int, str]:
         webhook = WorkItemWebhook()
         try:
             subscription, shared_secret = webhook.create_subscription(
@@ -432,7 +448,7 @@ class VstsIntegrationProvider(IntegrationProvider):
         subscription_id = subscription["id"]
         return subscription_id, shared_secret
 
-    def get_oauth_data(self, payload):
+    def get_oauth_data(self, payload: Mapping[str, Any]) -> Mapping[str, Any]:
         data = {"access_token": payload["access_token"]}
 
         if "expires_in" in payload:
@@ -445,7 +461,8 @@ class VstsIntegrationProvider(IntegrationProvider):
         return data
 
     @classmethod
-    def get_base_url(cls, access_token, account_id):
+    def get_base_url(cls, access_token: str, account_id: int) -> Optional[str]:
+        """TODO(mgaeta): This should not be allowed to return None."""
         url = VstsIntegrationProvider.VSTS_ACCOUNT_LOOKUP_URL % account_id
         with http.build_session() as session:
             response = session.get(
@@ -456,10 +473,12 @@ class VstsIntegrationProvider(IntegrationProvider):
                 },
             )
         if response.status_code == 200:
-            return response.json()["locationUrl"]
+            # Explicitly typing to satisfy mypy.
+            location_url: Optional[str] = response.json()["locationUrl"]
+            return location_url
         return None
 
-    def setup(self):
+    def setup(self) -> None:
         from sentry.plugins.base import bindings
 
         bindings.add(
@@ -467,8 +486,8 @@ class VstsIntegrationProvider(IntegrationProvider):
         )
 
 
-class AccountConfigView(PipelineView):
-    def dispatch(self, request, pipeline):
+class AccountConfigView(PipelineView):  # type: ignore
+    def dispatch(self, request: Request, pipeline: Pipeline) -> Response:
         if "account" in request.POST:
             account_id = request.POST.get("account")
             accounts = pipeline.fetch_state(key="accounts")
@@ -507,13 +526,15 @@ class AccountConfigView(PipelineView):
             request=request,
         )
 
-    def get_account_from_id(self, account_id, accounts):
+    def get_account_from_id(
+        self, account_id: int, accounts: Sequence[Mapping[str, Any]]
+    ) -> Optional[Mapping[str, Any]]:
         for account in accounts:
             if account["accountId"] == account_id:
                 return account
         return None
 
-    def get_accounts(self, access_token, user_id):
+    def get_accounts(self, access_token: str, user_id: int) -> Optional[JSONData]:
         url = (
             f"https://app.vssps.visualstudio.com/_apis/accounts?memberId={user_id}&api-version=4.1"
         )
@@ -530,8 +551,8 @@ class AccountConfigView(PipelineView):
         return None
 
 
-class AccountForm(forms.Form):
-    def __init__(self, accounts, *args, **kwargs):
+class AccountForm(forms.Form):  # type: ignore
+    def __init__(self, accounts: Sequence[Mapping[str, str]], *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
         self.fields["account"] = forms.ChoiceField(
             choices=[(acct["accountId"], acct["accountName"]) for acct in accounts],

+ 42 - 29
src/sentry/integrations/vsts/issues.py

@@ -1,15 +1,20 @@
 from collections import OrderedDict
+from typing import TYPE_CHECKING, Any, Mapping, MutableMapping, Optional, Sequence, Tuple
 
 from django.urls import reverse
 from django.utils.translation import ugettext as _
 from mistune import markdown
+from rest_framework.response import Response
 
 from sentry.integrations.issues import IssueSyncMixin
-from sentry.models import IntegrationExternalProject, OrganizationIntegration, User
+from sentry.models import Activity, IntegrationExternalProject, OrganizationIntegration, User
 from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized
 
+if TYPE_CHECKING:
+    from sentry.models import ExternalIssue, Group
 
-class VstsIssueSync(IssueSyncMixin):
+
+class VstsIssueSync(IssueSyncMixin):  # type: ignore
     description = "Integrate Azure DevOps work items by linking a project."
     slug = "vsts"
     conf_key = slug
@@ -17,15 +22,17 @@ class VstsIssueSync(IssueSyncMixin):
     issue_fields = frozenset(["id", "title", "url"])
     done_categories = frozenset(["Resolved", "Completed", "Closed"])
 
-    def get_persisted_default_config_fields(self):
+    def get_persisted_default_config_fields(self) -> Sequence[str]:
         return ["project", "work_item_type"]
 
-    def create_default_repo_choice(self, default_repo):
+    def create_default_repo_choice(self, default_repo: str) -> Tuple[str, str]:
         # default_repo should be the project_id
         project = self.get_client().get_project(self.instance, default_repo)
         return (project["id"], project["name"])
 
-    def get_project_choices(self, group=None, **kwargs):
+    def get_project_choices(
+        self, group: Optional["Group"] = None, **kwargs: Any
+    ) -> Tuple[Optional[str], Sequence[Tuple[str, str]]]:
         client = self.get_client()
         try:
             projects = client.get_projects(self.instance)
@@ -35,10 +42,10 @@ class VstsIssueSync(IssueSyncMixin):
         project_choices = [(project["id"], project["name"]) for project in projects]
 
         params = kwargs.get("params", {})
+        project = kwargs.get("project")
         if group:
             default_project_id = group.project_id
-        elif kwargs.get("project"):
-            project = kwargs.get("project")
+        elif project:
             default_project_id = project.id
         else:
             default_project_id = projects[0]["id"]
@@ -63,7 +70,9 @@ class VstsIssueSync(IssueSyncMixin):
 
         return default_project, project_choices
 
-    def get_work_item_choices(self, project, group=None):
+    def get_work_item_choices(
+        self, project: str, group: Optional["Group"] = None
+    ) -> Tuple[Optional[str], Sequence[Tuple[str, str]]]:
         client = self.get_client()
         try:
             item_categories = client.get_work_item_categories(self.instance, project)["value"]
@@ -93,10 +102,12 @@ class VstsIssueSync(IssueSyncMixin):
 
         return default_item_type, item_tuples
 
-    def get_create_issue_config_no_group(self, project):
+    def get_create_issue_config_no_group(self, project: str) -> Sequence[Mapping[str, Any]]:
         return self.get_create_issue_config(None, None, project=project)
 
-    def get_create_issue_config(self, group, user, **kwargs):
+    def get_create_issue_config(
+        self, group: Optional["Group"], user: Optional[User], **kwargs: Any
+    ) -> Sequence[Mapping[str, Any]]:
         kwargs["link_referrer"] = "vsts_integration"
         fields = []
         if group:
@@ -105,8 +116,8 @@ class VstsIssueSync(IssueSyncMixin):
             # Workitems (issues) are associated with the project not the repository.
         default_project, project_choices = self.get_project_choices(group, **kwargs)
 
-        work_item_choices = []
-        default_work_item = None
+        work_item_choices: Sequence[Tuple[str, str]] = []
+        default_work_item: Optional[str] = None
         if default_project:
             default_work_item, work_item_choices = self.get_work_item_choices(
                 default_project, group
@@ -134,8 +145,8 @@ class VstsIssueSync(IssueSyncMixin):
             },
         ] + fields
 
-    def get_link_issue_config(self, group, **kwargs):
-        fields = super().get_link_issue_config(group, **kwargs)
+    def get_link_issue_config(self, group: "Group", **kwargs: Any) -> Sequence[Mapping[str, str]]:
+        fields: Sequence[MutableMapping[str, str]] = super().get_link_issue_config(group, **kwargs)
         org = group.organization
         autocomplete_url = reverse("sentry-extensions-vsts-search", args=[org.slug, self.model.id])
         for field in fields:
@@ -144,10 +155,10 @@ class VstsIssueSync(IssueSyncMixin):
                 field["type"] = "select"
         return fields
 
-    def get_issue_url(self, key, **kwargs):
+    def get_issue_url(self, key: str, **kwargs: Any) -> str:
         return f"{self.instance}_workitems/edit/{key}"
 
-    def create_issue(self, data, **kwargs):
+    def create_issue(self, data: Mapping[str, str], **kwargs: Any) -> Mapping[str, Any]:
         """
         Creates the issue on the remote service and returns an issue ID.
         """
@@ -182,7 +193,7 @@ class VstsIssueSync(IssueSyncMixin):
             "metadata": {"display_name": "{}#{}".format(project_name, created_item["id"])},
         }
 
-    def get_issue(self, issue_id, **kwargs):
+    def get_issue(self, issue_id: str, **kwargs: Any) -> Mapping[str, Any]:
         client = self.get_client()
         work_item = client.get_work_item(self.instance, issue_id)
         return {
@@ -196,7 +207,9 @@ class VstsIssueSync(IssueSyncMixin):
             },
         }
 
-    def sync_assignee_outbound(self, external_issue, user, assign=True, **kwargs):
+    def sync_assignee_outbound(
+        self, external_issue: "ExternalIssue", user: User, assign: bool = True, **kwargs: Any
+    ) -> None:
         client = self.get_client()
         assignee = None
 
@@ -239,7 +252,9 @@ class VstsIssueSync(IssueSyncMixin):
                 },
             )
 
-    def sync_status_outbound(self, external_issue, is_resolved, project_id, **kwargs):
+    def sync_status_outbound(
+        self, external_issue: "ExternalIssue", is_resolved: bool, project_id: int, **kwargs: Any
+    ) -> None:
         client = self.get_client()
         work_item = client.get_work_item(self.instance, external_issue.key)
         # For some reason, vsts doesn't include the project id
@@ -291,15 +306,15 @@ class VstsIssueSync(IssueSyncMixin):
                 },
             )
 
-    def should_unresolve(self, data):
+    def should_unresolve(self, data: Mapping[str, str]) -> bool:
         done_states = self.get_done_states(data["project"])
         return not data["new_state"] in done_states or data["old_state"] is None
 
-    def should_resolve(self, data):
+    def should_resolve(self, data: Mapping[str, str]) -> bool:
         done_states = self.get_done_states(data["project"])
         return not data["old_state"] in done_states and data["new_state"] in done_states
 
-    def get_done_states(self, project):
+    def get_done_states(self, project: str) -> Sequence[str]:
         client = self.get_client()
         try:
             all_states = client.get_work_item_states(self.instance, project)["value"]
@@ -314,17 +329,15 @@ class VstsIssueSync(IssueSyncMixin):
         ]
         return done_states
 
-    def get_issue_display_name(self, external_issue):
-        if external_issue.metadata is None:
-            return ""
-        return external_issue.metadata["display_name"]
+    def get_issue_display_name(self, external_issue: "ExternalIssue") -> str:
+        return (external_issue.metadata or {}).get("display_name", "")
 
-    def create_comment(self, issue_id, user_id, group_note):
+    def create_comment(self, issue_id: str, user_id: int, group_note: Activity) -> Response:
         comment = group_note.data["text"]
         quoted_comment = self.create_comment_attribution(user_id, comment)
         return self.get_client().update_work_item(self.instance, issue_id, comment=quoted_comment)
 
-    def create_comment_attribution(self, user_id, comment_text):
+    def create_comment_attribution(self, user_id: int, comment_text: str) -> str:
         # VSTS uses markdown or xml
         # https://docs.microsoft.com/en-us/microsoftteams/platform/concepts/bots/bots-text-formats
         user = User.objects.get(id=user_id)
@@ -332,6 +345,6 @@ class VstsIssueSync(IssueSyncMixin):
         quoted_comment = f"{attribution}<blockquote>{comment_text}</blockquote>"
         return quoted_comment
 
-    def update_comment(self, issue_id, user_id, group_note):
+    def update_comment(self, issue_id: int, user_id: int, group_note: str) -> None:
         # Azure does not support updating comments.
         pass

+ 5 - 3
src/sentry/integrations/vsts/notify_action.py

@@ -1,5 +1,7 @@
 import logging
+from typing import Any
 
+from sentry.eventstore.models import Event
 from sentry.rules.actions.base import TicketEventAction
 from sentry.utils.http import absolute_uri
 from sentry.web.decorators import transaction_start
@@ -7,19 +9,19 @@ from sentry.web.decorators import transaction_start
 logger = logging.getLogger("sentry.rules")
 
 
-class AzureDevopsCreateTicketAction(TicketEventAction):
+class AzureDevopsCreateTicketAction(TicketEventAction):  # type: ignore
     label = "Create an Azure DevOps work item in {integration} with these "
     ticket_type = "an Azure DevOps work item"
     link = "https://docs.sentry.io/product/integrations/source-code-mgmt/azure-devops/#issue-sync"
     provider = "vsts"
     integration_key = "integration"
 
-    def generate_footer(self, rule_url):
+    def generate_footer(self, rule_url: str) -> str:
         return "\nThis work item was automatically created by Sentry via [{}]({})".format(
             self.rule.label,
             absolute_uri(rule_url),
         )
 
     @transaction_start("AzureDevopsCreateTicketAction.after")
-    def after(self, event, state):
+    def after(self, event: Event, state: str) -> Any:
         yield super().after(event, state)

+ 33 - 12
src/sentry/integrations/vsts/repository.py

@@ -1,17 +1,21 @@
 import logging
+from typing import Any, Mapping, MutableMapping, Optional, Sequence
 
-from sentry.models import Integration
+from sentry.integrations import IntegrationInstallation
+from sentry.models import Commit, Integration, Organization, Repository
 from sentry.plugins import providers
 from sentry.shared_integrations.exceptions import IntegrationError
 
 MAX_COMMIT_DATA_REQUESTS = 90
 
 
-class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
+class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):  # type: ignore
     name = "Azure DevOps"
     logger = logging.getLogger("sentry.integrations.vsts")
 
-    def get_installation(self, integration_id, organization_id):
+    def get_installation(
+        self, integration_id: Optional[int], organization_id: int
+    ) -> IntegrationInstallation:
         if integration_id is None:
             raise IntegrationError("%s requires an integration id." % self.name)
 
@@ -19,9 +23,13 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
             id=integration_id, organizations=organization_id, provider="vsts"
         )
 
-        return integration_model.get_installation(organization_id)
+        # Explicitly typing to satisfy mypy.
+        installation: IntegrationInstallation = integration_model.get_installation(organization_id)
+        return installation
 
-    def get_repository_data(self, organization, config):
+    def get_repository_data(
+        self, organization: Organization, config: MutableMapping[str, Any]
+    ) -> Mapping[str, str]:
         installation = self.get_installation(config.get("installation"), organization.id)
         client = installation.get_client()
         instance = installation.instance
@@ -43,7 +51,9 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
         )
         return config
 
-    def build_repository_config(self, organization, data):
+    def build_repository_config(
+        self, organization: Organization, data: Mapping[str, str]
+    ) -> Mapping[str, Any]:
         return {
             "name": data["name"],
             "external_id": data["external_id"],
@@ -56,7 +66,9 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
             "integration_id": data["installation"],
         }
 
-    def transform_changes(self, patch_set):
+    def transform_changes(
+        self, patch_set: Sequence[Mapping[str, Any]]
+    ) -> Sequence[Mapping[str, str]]:
         type_mapping = {"add": "A", "delete": "D", "edit": "M"}
         file_changes = []
         # https://docs.microsoft.com/en-us/rest/api/vsts/git/commits/get%20changes#versioncontrolchangetype
@@ -68,7 +80,9 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
 
         return file_changes
 
-    def zip_commit_data(self, repo, commit_list, organization_id):
+    def zip_commit_data(
+        self, repo: Repository, commit_list: Sequence[Commit], organization_id: int
+    ) -> Sequence[Commit]:
         installation = self.get_installation(repo.integration_id, organization_id)
         client = installation.get_client()
         n = 0
@@ -95,7 +109,10 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
 
         return commit_list
 
-    def compare_commits(self, repo, start_sha, end_sha):
+    def compare_commits(
+        self, repo: Repository, start_sha: Optional[str], end_sha: str
+    ) -> Sequence[Mapping[str, str]]:
+        """TODO(mgaeta): This function is kinda a mess."""
         installation = self.get_installation(repo.integration_id, repo.organization_id)
         client = installation.get_client()
         instance = repo.config["instance"]
@@ -111,7 +128,9 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
         commits = self.zip_commit_data(repo, res["value"], repo.organization_id)
         return self._format_commits(repo, commits)
 
-    def _format_commits(self, repo, commit_list):
+    def _format_commits(
+        self, repo: Repository, commit_list: Sequence[Mapping[str, Any]]
+    ) -> Sequence[Mapping[str, Any]]:
         return [
             {
                 "id": c["commitId"],
@@ -125,5 +144,7 @@ class VstsRepositoryProvider(providers.IntegrationRepositoryProvider):
             for c in commit_list
         ]
 
-    def repository_external_slug(self, repo):
-        return repo.external_id
+    def repository_external_slug(self, repo: Repository) -> str:
+        # Explicitly typing to satisfy mypy.
+        external_id: str = repo.external_id
+        return external_id

+ 4 - 3
src/sentry/integrations/vsts/search.py

@@ -1,11 +1,12 @@
+from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry.api.bases.integration import IntegrationEndpoint
-from sentry.models import Integration
+from sentry.models import Integration, Organization
 
 
-class VstsSearchEndpoint(IntegrationEndpoint):
-    def get(self, request, organization, integration_id):
+class VstsSearchEndpoint(IntegrationEndpoint):  # type: ignore
+    def get(self, request: Request, organization: Organization, integration_id: int) -> Response:
         try:
             integration = Integration.objects.get(
                 organizations=organization, id=integration_id, provider="vsts"

+ 34 - 17
src/sentry/integrations/vsts/webhooks.py

@@ -1,8 +1,11 @@
 import logging
 import re
+from typing import Any, Mapping, Optional
 
 from django.utils.crypto import constant_time_compare
 from django.views.decorators.csrf import csrf_exempt
+from rest_framework.request import Request
+from rest_framework.response import Response
 
 from sentry.api.base import Endpoint
 from sentry.models import (
@@ -22,18 +25,18 @@ logger = logging.getLogger("sentry.integrations")
 PROVIDER_KEY = "vsts"
 
 
-class WorkItemWebhook(Endpoint):
+class WorkItemWebhook(Endpoint):  # type: ignore
     authentication_classes = ()
     permission_classes = ()
 
-    def get_client(self, identity, oauth_redirect_url):
+    def get_client(self, identity: Identity, oauth_redirect_url: str) -> VstsApiClient:
         return VstsApiClient(identity, oauth_redirect_url)
 
-    @csrf_exempt
-    def dispatch(self, request, *args, **kwargs):
+    @csrf_exempt  # type: ignore
+    def dispatch(self, request: Request, *args: Any, **kwargs: Any) -> Response:
         return super().dispatch(request, *args, **kwargs)
 
-    def post(self, request, *args, **kwargs):
+    def post(self, request: Request, *args: Any, **kwargs: Any) -> Response:
         data = request.data
         try:
             event_type = data["eventType"]
@@ -69,7 +72,7 @@ class WorkItemWebhook(Endpoint):
             self.handle_updated_workitem(data, integration)
         return self.respond()
 
-    def check_webhook_secret(self, request, integration):
+    def check_webhook_secret(self, request: Request, integration: Integration) -> None:
         try:
             integration_secret = integration.metadata["subscription"]["secret"]
             webhook_payload_secret = request.META["HTTP_SHARED_SECRET"]
@@ -90,8 +93,8 @@ class WorkItemWebhook(Endpoint):
 
         assert constant_time_compare(integration_secret, webhook_payload_secret)
 
-    def handle_updated_workitem(self, data, integration):
-        project = None
+    def handle_updated_workitem(self, data: Mapping[str, Any], integration: Integration) -> None:
+        project: Optional[str] = None
         try:
             external_issue_key = data["resource"]["workItemId"]
             project = data["resourceContainers"]["project"]["id"]
@@ -126,9 +129,18 @@ class WorkItemWebhook(Endpoint):
         self.handle_assign_to(integration, external_issue_key, assigned_to)
         self.handle_status_change(integration, external_issue_key, status_change, project)
 
-    def handle_assign_to(self, integration, external_issue_key, assigned_to):
+    def handle_assign_to(
+        self,
+        integration: Integration,
+        external_issue_key: str,
+        assigned_to: Optional[Mapping[str, str]],
+    ) -> None:
         if not assigned_to:
             return
+
+        email: Optional[str] = None
+        assign = False
+
         new_value = assigned_to.get("newValue")
         if new_value is not None:
             try:
@@ -145,9 +157,6 @@ class WorkItemWebhook(Endpoint):
                 )
                 return  # TODO(lb): return if cannot parse email?
             assign = True
-        else:
-            email = None
-            assign = False
 
         sync_group_assignee_inbound(
             integration=integration,
@@ -156,7 +165,13 @@ class WorkItemWebhook(Endpoint):
             assign=assign,
         )
 
-    def handle_status_change(self, integration, external_issue_key, status_change, project):
+    def handle_status_change(
+        self,
+        integration: Integration,
+        external_issue_key: str,
+        status_change: Optional[Mapping[str, str]],
+        project: Optional[str],
+    ) -> None:
         if status_change is None:
             return
 
@@ -175,11 +190,13 @@ class WorkItemWebhook(Endpoint):
 
             installation.sync_status_inbound(external_issue_key, data)
 
-    def parse_email(self, email):
-        # TODO(lb): hmm... this looks brittle to me
-        return EMAIL_PARSER.search(email).group(1)
+    def parse_email(self, email: str) -> str:
+        # TODO(mgaeta): This is too brittle and doesn't pass types.
+        return EMAIL_PARSER.search(email).group(1)  # type: ignore
 
-    def create_subscription(self, instance, identity_data, oauth_redirect_url):
+    def create_subscription(
+        self, instance: Optional[str], identity_data: Mapping[str, Any], oauth_redirect_url: str
+    ) -> Response:
         client = self.get_client(Identity(data=identity_data), oauth_redirect_url)
         shared_secret = generate_token()
         return client.create_subscription(instance, shared_secret), shared_secret