Skip to content

Commit 7fe516c

Browse files
committed
ref(vsts): Remove legacy web setup views in favor of API pipeline steps
VSTS installs now run through `get_pipeline_api_steps()` (`OAuth2ApiStep` + `VstsAccountSelectionApiStep`), which the frontend pipeline modal already drives. `VstsAccountSelectionApiStep` is a direct replacement for the old `AccountConfigView`, and in API mode the OAuth callback returns via the popup -> pipeline endpoint rather than the web `PipelineAdvancerView`, so the server-rendered views are dead. Drop the `AccountConfigView`/`AccountForm` classes and the `NestedPipelineView`-based `get_pipeline_views()` (now an empty list), and migrate the install tests/fixtures to the API pipeline flow. `get_pipeline_views()` still returns an empty list because the base provider requires it; removing it across all providers is a follow-up.
1 parent 0b7e908 commit 7fe516c

4 files changed

Lines changed: 94 additions & 270 deletions

File tree

fixtures/vsts.py

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
from __future__ import annotations
22

33
from typing import Any
4-
from urllib.parse import parse_qs, urlencode, urlparse
4+
from urllib.parse import parse_qs, urlparse
55

66
import pytest
77
import responses
8+
from django.urls import reverse
89

910
from sentry.integrations.models.integration import Integration
1011
from sentry.integrations.vsts import VstsIntegrationProvider
@@ -194,49 +195,45 @@ def _stub_vsts(self):
194195
},
195196
)
196197

197-
def make_init_request(self, path=None, body=None):
198-
return self.client.get(path or self.init_path, body or {})
199-
200-
def make_oauth_redirect_request(self, state):
201-
return self.client.get(
202-
"{}?{}".format(self.setup_path, urlencode({"code": "oauth-code", "state": state}))
198+
def _pipeline_url(self):
199+
return reverse(
200+
"sentry-api-0-organization-pipeline",
201+
kwargs={
202+
"organization_id_or_slug": self.organization.slug,
203+
"pipeline_name": "integration_pipeline",
204+
},
203205
)
204206

205-
def assert_vsts_oauth_redirect(self, redirect):
206-
assert redirect.scheme == "https"
207-
assert redirect.netloc == "app.vssps.visualstudio.com"
208-
assert redirect.path == "/oauth2/authorize"
209-
210207
def assert_vsts_new_oauth_redirect(self, redirect):
211208
assert redirect.scheme == "https"
212209
assert redirect.netloc == "login.microsoftonline.com"
213210
assert redirect.path == "/common/oauth2/v2.0/authorize"
214211

215-
def assert_account_selection(self, response, account_id=None):
212+
@assume_test_silo_mode(SiloMode.CONTROL)
213+
def assert_installation(self, account_id=None):
214+
# Drives the API pipeline: initialize -> oauth -> account selection.
216215
account_id = account_id or self.vsts_account_id
217-
assert response.status_code == 200
218-
assert f'<option value="{account_id}"'.encode() in response.content
216+
pipeline_url = self._pipeline_url()
219217

220-
@assume_test_silo_mode(SiloMode.CONTROL)
221-
def assert_installation(self):
222-
# Initial request to the installation URL for VSTS
223-
resp = self.make_init_request()
224-
redirect = urlparse(resp["Location"])
218+
resp: Any = self.client.post(
219+
pipeline_url, data={"action": "initialize", "provider": "vsts"}, format="json"
220+
)
221+
assert resp.status_code == 200, resp.content
225222

226-
assert resp.status_code == 302
223+
# The OAuth authorize URL carries the pipeline state signature, which is
224+
# echoed back to the advance step to verify the callback.
225+
oauth_url = resp.data["data"]["oauthUrl"]
226+
redirect = urlparse(oauth_url)
227227
self.assert_vsts_new_oauth_redirect(redirect)
228+
state = parse_qs(redirect.query)["state"][0]
228229

229-
query = parse_qs(redirect.query)
230-
231-
# OAuth redirect back to Sentry (identity_pipeline_view)
232-
resp = self.make_oauth_redirect_request(query["state"][0])
233-
self.assert_account_selection(resp)
234-
235-
# User choosing which VSTS Account to use (AccountConfigView)
236-
# Final step.
237230
resp = self.client.post(
238-
self.setup_path, {"account": self.vsts_account_id, "provider": "vsts"}
231+
pipeline_url, data={"code": "oauth-code", "state": state}, format="json"
239232
)
233+
assert resp.status_code == 200, resp.content
234+
assert resp.data["step"] == "account_selection"
235+
236+
resp = self.client.post(pipeline_url, data={"account": account_id}, format="json")
240237
return resp
241238

242239

src/sentry/integrations/vsts/integration.py

Lines changed: 2 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
from typing import Any, TypedDict
88
from urllib.parse import parse_qs, quote, unquote, urlencode, urlparse
99

10-
from django import forms
1110
from django.http.request import HttpRequest
12-
from django.http.response import HttpResponseBase
1311
from django.utils.translation import gettext as _
1412
from rest_framework import serializers
1513
from rest_framework.fields import CharField
@@ -20,7 +18,6 @@
2018
from sentry.auth.exceptions import IdentityNotValid
2119
from sentry.constants import ObjectStatus
2220
from sentry.identity.oauth2 import OAuth2ApiStep
23-
from sentry.identity.pipeline import IdentityPipeline
2421
from sentry.identity.services.identity.model import RpcIdentity
2522
from sentry.identity.vsts.provider import VSTSNewIdentityProvider, get_user_info
2623
from sentry.integrations.base import (
@@ -55,7 +52,6 @@
5552
from sentry.organizations.services.organization.model import RpcOrganization
5653
from sentry.pipeline.types import PipelineStepResult
5754
from sentry.pipeline.views.base import ApiPipelineSteps, PipelineView
58-
from sentry.pipeline.views.nested import NestedPipelineView
5955
from sentry.shared_integrations.exceptions import (
6056
ApiError,
6157
IntegrationError,
@@ -64,7 +60,6 @@
6460
from sentry.silo.base import SiloMode
6561
from sentry.utils import metrics
6662
from sentry.utils.http import absolute_uri
67-
from sentry.web.helpers import render_to_response
6863

6964
from .client import VstsApiClient, VstsSetupApiClient
7065
from .repository import VstsRepositoryProvider
@@ -611,20 +606,7 @@ def get_scopes(self) -> Sequence[str]:
611606
return VstsIntegrationProvider.NEW_SCOPES
612607

613608
def get_pipeline_views(self) -> Sequence[PipelineView[IntegrationPipeline]]:
614-
identity_pipeline_config = {
615-
"redirect_url": absolute_uri(self.oauth_redirect_url),
616-
"oauth_scopes": self.get_scopes(),
617-
}
618-
619-
return [
620-
NestedPipelineView(
621-
bind_key="identity",
622-
provider_key=self.key,
623-
pipeline_cls=IdentityPipeline,
624-
config=identity_pipeline_config,
625-
),
626-
AccountConfigView(),
627-
]
609+
return []
628610

629611
def get_pipeline_api_steps(self) -> ApiPipelineSteps[IntegrationPipeline]:
630612
return [
@@ -650,13 +632,7 @@ def _make_oauth_api_step(self) -> OAuth2ApiStep:
650632
)
651633

652634
def build_integration(self, state: Mapping[str, Any]) -> IntegrationData:
653-
# TODO: legacy views write token data to state["identity"]["data"] via
654-
# NestedPipelineView. API steps write directly to state["oauth_data"].
655-
# Remove the legacy path once the old views are retired.
656-
if "oauth_data" in state:
657-
data = state["oauth_data"]
658-
else:
659-
data = state["identity"]["data"]
635+
data = state["oauth_data"]
660636
oauth_data = self.get_oauth_data(data)
661637
account = state["account"]
662638
user = get_user_info(data["access_token"])
@@ -834,57 +810,3 @@ def setup(self) -> None:
834810
bindings.add(
835811
"integration-repository.provider", VstsRepositoryProvider, id="integrations:vsts"
836812
)
837-
838-
839-
class AccountConfigView:
840-
def dispatch(self, request: HttpRequest, pipeline: IntegrationPipeline) -> HttpResponseBase:
841-
with IntegrationPipelineViewEvent(
842-
IntegrationPipelineViewType.ACCOUNT_CONFIG,
843-
IntegrationDomain.SOURCE_CODE_MANAGEMENT,
844-
VstsIntegrationProvider.key,
845-
).capture() as lifecycle:
846-
account_id = request.POST.get("account")
847-
if account_id is not None:
848-
state_accounts: Sequence[Mapping[str, Any]] | None = pipeline.fetch_state(
849-
key="accounts"
850-
)
851-
account = get_account_from_id(account_id, state_accounts or [])
852-
if account is not None:
853-
pipeline.bind_state("account", account)
854-
return pipeline.next_step()
855-
856-
state: Mapping[str, Any] | None = pipeline.fetch_state(key="identity")
857-
access_token = (state or {}).get("data", {}).get("access_token")
858-
user = get_user_info(access_token)
859-
860-
accounts = get_accounts(access_token, user["uuid"])
861-
extra = {
862-
"organization_id": pipeline.organization.id if pipeline.organization else None,
863-
"user_id": request.user.id,
864-
"accounts": accounts,
865-
}
866-
if not accounts or not accounts.get("value"):
867-
lifecycle.record_halt(IntegrationPipelineHaltReason.NO_ACCOUNTS, extra=extra)
868-
return render_to_response(
869-
template="sentry/integrations/vsts-config.html",
870-
context={"no_accounts": True},
871-
request=request,
872-
)
873-
accounts = accounts["value"]
874-
pipeline.bind_state("accounts", accounts)
875-
account_form = AccountForm(accounts)
876-
return render_to_response(
877-
template="sentry/integrations/vsts-config.html",
878-
context={"form": account_form, "no_accounts": False},
879-
request=request,
880-
)
881-
882-
883-
class AccountForm(forms.Form):
884-
def __init__(self, accounts: Sequence[Mapping[str, str]], *args: Any, **kwargs: Any) -> None:
885-
super().__init__(*args, **kwargs)
886-
self.fields["account"] = forms.ChoiceField(
887-
choices=[(acct["accountId"], acct["accountName"]) for acct in accounts],
888-
label="Account",
889-
help_text="Azure DevOps organization.",
890-
)

0 commit comments

Comments
 (0)