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

Conversation

rbro112
Copy link
Member

@rbro112 rbro112 commented Aug 8, 2025

Add new vcs fields to buildDetails API

Nullable fields

Copy link
Member Author

rbro112 commented Aug 8, 2025

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 8, 2025
Comment on lines +129 to +168
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
),
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.

Copy link

codecov bot commented Aug 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #97456      +/-   ##
==========================================
- Coverage   80.58%   80.58%   -0.01%     
==========================================
  Files        8557     8557              
  Lines      376468   376553      +85     
  Branches    24468    24468              
==========================================
+ Hits       303381   303437      +56     
- Misses      72717    72746      +29     
  Partials      370      370              

@rbro112 rbro112 changed the title Add new vcs fields to buildDetails API feat(emerge): Add new vcs fields to buildDetails API Aug 8, 2025
@rbro112 rbro112 force-pushed the ryan/add_new_vcs_related_fields_to_preprod_artifact_assemble_endpoint branch from a5b687e to de6b5e0 Compare August 8, 2025 02:27
@rbro112 rbro112 force-pushed the ryan/add_new_vcs_fields_to_builddetails_api branch from feac969 to 94ac7bd Compare August 8, 2025 02:27
@rbro112 rbro112 force-pushed the ryan/add_new_vcs_fields_to_builddetails_api branch from 94ac7bd to 91c2113 Compare August 8, 2025 02:54
Base automatically changed from ryan/add_new_vcs_related_fields_to_preprod_artifact_assemble_endpoint to master August 8, 2025 03:19
@rbro112 rbro112 force-pushed the ryan/add_new_vcs_fields_to_builddetails_api branch from 91c2113 to 5875175 Compare August 8, 2025 15:51
@rbro112 rbro112 merged commit cbf33d9 into master Aug 8, 2025
65 checks passed
@rbro112 rbro112 deleted the ryan/add_new_vcs_fields_to_builddetails_api branch August 8, 2025 16:15
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
Add new vcs fields to buildDetails API

Nullable fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants