Skip to content

feat(emerge): Add new vcs fields to buildDetails API #97456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,46 @@ def get(self, request: Request, project, artifact_id) -> Response:
)

vcs_info = BuildDetailsVcsInfo(
commit_id=preprod_artifact.commit.key if preprod_artifact.commit else None,
# TODO: Implement in the future when available
# repo=None,
# provider=None,
# branch=None,
head_sha=(
preprod_artifact.commit_comparison.head_sha
if preprod_artifact.commit_comparison
else None
),
base_sha=(
preprod_artifact.commit_comparison.base_sha
if preprod_artifact.commit_comparison
else None
),
provider=(
preprod_artifact.commit_comparison.provider
if preprod_artifact.commit_comparison
else None
),
head_repo_name=(
preprod_artifact.commit_comparison.head_repo_name
if preprod_artifact.commit_comparison
else None
),
base_repo_name=(
preprod_artifact.commit_comparison.base_repo_name
if preprod_artifact.commit_comparison
else None
),
head_ref=(
preprod_artifact.commit_comparison.head_ref
if preprod_artifact.commit_comparison
else None
),
base_ref=(
preprod_artifact.commit_comparison.base_ref
if preprod_artifact.commit_comparison
else None
),
pr_number=(
preprod_artifact.commit_comparison.pr_number
if preprod_artifact.commit_comparison
else None
),
Comment on lines +129 to +168
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: The API response removes the `commit_id` field, but frontend code and tests still expect it, which will cause test failures and break UI components.
  • Description: The ProjectPreprodBuildDetailsEndpoint is modified to no longer include the commit_id field in the vcs_info dictionary of its API response. However, consumers of this endpoint have not been updated to reflect this change. Tests in test_project_preprod_build_details.py explicitly assert the presence of this key, which will cause them to fail. Furthermore, the frontend component buildDetailsSidebarContent.tsx accesses buildDetailsData.vcs_info?.commit_id, and its corresponding TypeScript interface expects the field to exist. Removing this field is a breaking API change that will cause test failures and result in incorrect data being displayed in the user interface.
  • Suggested fix: To maintain backward compatibility, re-introduce the commit_id field to the API response. Alternatively, update all consumers, including tests and frontend code, to use the new fields (e.g., head_sha) and remove their dependency on commit_id.
    severity: 0.85, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

)

api_response = BuildDetailsApiResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,14 @@ class BuildDetailsAppInfo(BaseModel):


class BuildDetailsVcsInfo(BaseModel):
commit_id: str | None = None
# repo: Optional[str] = None # Uncomment when available
# provider: Optional[str] = None # Uncomment when available
# branch: Optional[str] = None # Uncomment when available
head_sha: str | None = None
base_sha: str | None = None
provider: str | None = None
head_repo_name: str | None = None
base_repo_name: str | None = None
head_ref: str | None = None
base_ref: str | None = None
pr_number: int | None = None


class BuildDetailsSizeInfo(BaseModel):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.urls import reverse

from sentry.models.commit import Commit
from sentry.models.commitcomparison import CommitComparison
from sentry.preprod.models import PreprodArtifact
from sentry.testutils.cases import APITestCase

Expand All @@ -18,10 +18,16 @@ def setUp(self) -> None:

self.file = self.create_file(name="test_artifact.apk", type="application/octet-stream")

self.commit = Commit.objects.create(
commit_comparison = CommitComparison.objects.create(
organization_id=self.org.id,
repository_id=1,
key="abcdef1234567890",
head_sha="1234567890098765432112345678900987654321",
base_sha="9876543210012345678998765432100123456789",
provider="github",
head_repo_name="owner/repo",
base_repo_name="owner/repo",
head_ref="feature/xyz",
base_ref="main",
pr_number=123,
)

self.preprod_artifact = PreprodArtifact.objects.create(
Expand All @@ -35,7 +41,7 @@ def setUp(self) -> None:
build_number=42,
build_configuration_id=None,
installable_app_file_id=1234,
commit=self.commit,
commit_comparison=commit_comparison,
)

# Enable the feature flag for all tests by default
Expand Down Expand Up @@ -68,9 +74,6 @@ def test_get_build_details_success(self) -> None:
assert resp_data["app_info"]["version"] == self.preprod_artifact.build_version
assert resp_data["app_info"]["build_number"] == self.preprod_artifact.build_number
assert resp_data["app_info"]["artifact_type"] == self.preprod_artifact.artifact_type
assert resp_data["vcs_info"]["commit_id"] == (
self.preprod_artifact.commit.key if self.preprod_artifact.commit is not None else None
)

def test_get_build_details_not_found(self) -> None:
url = self._get_url(artifact_id=999999)
Expand Down Expand Up @@ -108,12 +111,6 @@ def test_get_build_details_dates_and_types(self) -> None:
assert isinstance(resp_data["app_info"]["artifact_type"], int)

def test_get_build_details_vcs_info(self) -> None:
new_commit = Commit.objects.create(
organization_id=self.org.id,
repository_id=1,
key="deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
)
self.preprod_artifact.commit = new_commit
self.preprod_artifact.save()

url = self._get_url()
Expand All @@ -122,4 +119,11 @@ def test_get_build_details_vcs_info(self) -> None:
)
assert response.status_code == 200
resp_data = response.json()
assert resp_data["vcs_info"]["commit_id"] == "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
assert resp_data["vcs_info"]["head_sha"] == "1234567890098765432112345678900987654321"
assert resp_data["vcs_info"]["base_sha"] == "9876543210012345678998765432100123456789"
assert resp_data["vcs_info"]["provider"] == "github"
assert resp_data["vcs_info"]["head_repo_name"] == "owner/repo"
assert resp_data["vcs_info"]["base_repo_name"] == "owner/repo"
assert resp_data["vcs_info"]["head_ref"] == "feature/xyz"
assert resp_data["vcs_info"]["base_ref"] == "main"
assert resp_data["vcs_info"]["pr_number"] == 123
Loading