diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index f1f043ca119b75..a5d26310dfa543 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -225,8 +225,6 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:preprod-size-monitors-frontend", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable preprod snapshots product feature manager.add("organizations:preprod-snapshots", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) - # Enables PR page - manager.add("organizations:pr-page", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enables the playstation ingestion in relay manager.add("organizations:relay-playstation-ingestion", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False) # Enables new error processing pipeline in Relay. diff --git a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_comments.py b/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_comments.py deleted file mode 100644 index 5e37a07194f09e..00000000000000 --- a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_comments.py +++ /dev/null @@ -1,210 +0,0 @@ -from __future__ import annotations - -import logging -from typing import Any - -from rest_framework.request import Request -from rest_framework.response import Response - -from sentry import analytics, features -from sentry.api.api_owners import ApiOwner -from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import OrganizationEndpoint -from sentry.integrations.github.client import GitHubBaseClient -from sentry.integrations.source_code_management.metrics import ( - SCMIntegrationInteractionEvent, - SCMIntegrationInteractionType, -) -from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization -from sentry.preprod.analytics import PreprodApiPrPageCommentsEvent -from sentry.preprod.integration_utils import get_github_client -from sentry.preprod.pull_request.comment_types import ( - IssueComment, - PullRequestComments, - PullRequestCommentsErrorResponse, - ReviewComment, -) -from sentry.shared_integrations.exceptions import ApiError - -logger = logging.getLogger(__name__) - - -@cell_silo_endpoint -class OrganizationPrCommentsEndpoint(OrganizationEndpoint): - owner = ApiOwner.EMERGE_TOOLS - publish_status = { - "GET": ApiPublishStatus.EXPERIMENTAL, - } - - def get( - self, request: Request, organization: Organization, repo_name: str, pr_number: str - ) -> Response: - """ - Get GitHub comments for a Pull Request. - - Returns both general PR comments and file-specific review comments. - - **Path Parameters:** - - `repo_name`: Repository name (e.g., 'owner/repo') - - `pr_number`: Pull request number - - **Example:** - ``` - GET /projects/sentry/pr-comments/getsentry/sentry/12345/ - ``` - """ - analytics.record( - PreprodApiPrPageCommentsEvent( - organization_id=organization.id, - user_id=request.user.id, - repo_name=repo_name, - pr_number=pr_number, - ) - ) - - if not features.has("organizations:pr-page", organization, actor=request.user): - return Response({"error": "Feature not enabled"}, status=403) - - client = get_github_client(organization, repo_name) - if not client: - logger.warning( - "No GitHub client found for organization", - extra={"organization_id": organization.id}, - ) - error_data = PullRequestCommentsErrorResponse( - error="integration_not_found", - message="No GitHub integration found for this repository", - details="Unable to find a GitHub integration for the specified repository", - ) - return Response(error_data.dict(), status=404) - - try: - general_comments_raw = self._fetch_pr_general_comments( - organization.id, client, repo_name, pr_number - ) - review_comments_raw = self._fetch_pr_review_comments( - organization.id, client, repo_name, pr_number - ) - - # Parse general comments - general_comments = [IssueComment.parse_obj(c) for c in general_comments_raw] - - # Parse and organize review comments by file path - file_comments: dict[str, list[ReviewComment]] = {} - for comment_data in review_comments_raw: - if "path" not in comment_data: - continue - - review_comment = ReviewComment.parse_obj(comment_data) - file_path = review_comment.path - - if file_path not in file_comments: - file_comments[file_path] = [] - file_comments[file_path].append(review_comment) - - comments_data = PullRequestComments( - general_comments=general_comments, - file_comments=file_comments, - ) - - logger.info( - "Fetched PR comments from GitHub", - extra={ - "organization_id": organization.id, - "repo_name": repo_name, - "pr_number": pr_number, - "general_comments_count": len(comments_data.general_comments), - "review_comments_count": sum( - len(comments) for comments in comments_data.file_comments.values() - ), - "files_with_comments": len(comments_data.file_comments), - }, - ) - - return Response(comments_data.dict(), status=200) - - except ApiError: - logger.exception( - "GitHub API error when fetching PR comments", - extra={ - "organization_id": organization.id, - "repo_name": repo_name, - "pr_number": pr_number, - }, - ) - error_data = PullRequestCommentsErrorResponse( - error="api_error", - message="Failed to fetch pull request comments from GitHub", - details="A problem occurred when communicating with GitHub. Please try again later.", - ) - return Response(error_data.dict(), status=502) - except Exception: - logger.exception( - "Unexpected error fetching PR comments", - extra={ - "organization_id": organization.id, - "repo_name": repo_name, - "pr_number": pr_number, - }, - ) - error_data = PullRequestCommentsErrorResponse( - error="internal_error", - message="An unexpected error occurred while fetching pull request comments", - ) - return Response(error_data.dict(), status=500) - - def _fetch_pr_general_comments( - self, - organization_id: int, - client: GitHubBaseClient, - repo_name: str, - pr_number: str, - ) -> list[dict[str, Any]]: - """ - Fetch general PR comments from GitHub. - - These are the comments posted in the main PR conversation thread. - """ - with SCMIntegrationInteractionEvent( - interaction_type=SCMIntegrationInteractionType.GET_ISSUE_COMMENTS, - provider_key=IntegrationProviderSlug.GITHUB.value, # only Github for now - organization_id=organization_id, - integration_id=client.integration_id, - ).capture() as lifecycle: - lifecycle.add_extras( - { - "repo_name": repo_name, - "pr_number": pr_number, - } - ) - comments = client.get_issue_comments(repo_name, pr_number) - return comments or [] - - def _fetch_pr_review_comments( - self, - organization_id: int, - client: GitHubBaseClient, - repo_name: str, - pr_number: str, - ) -> list[dict[str, Any]]: - """ - Fetch PR review comments from GitHub. - - These are the comments posted on specific lines in file diffs during code review. - """ - with SCMIntegrationInteractionEvent( - interaction_type=SCMIntegrationInteractionType.GET_PR_COMMENTS, - provider_key=IntegrationProviderSlug.GITHUB.value, # only Github for now - organization_id=organization_id, - integration_id=client.integration_id, - ).capture() as lifecycle: - lifecycle.add_extras( - { - "repo_name": repo_name, - "pr_number": pr_number, - } - ) - comments = client.get_pull_request_comments(repo_name, pr_number) - return comments or [] diff --git a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_details.py b/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_details.py deleted file mode 100644 index 459323e7ae0bbb..00000000000000 --- a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_details.py +++ /dev/null @@ -1,109 +0,0 @@ -from __future__ import annotations - -import logging - -from rest_framework.request import Request -from rest_framework.response import Response - -from sentry import analytics, features -from sentry.api.api_owners import ApiOwner -from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import OrganizationEndpoint -from sentry.models.organization import Organization -from sentry.preprod.analytics import PreprodApiPrPageDetailsEvent -from sentry.preprod.integration_utils import get_github_client -from sentry.preprod.pull_request.adapters import PullRequestDataAdapter -from sentry.preprod.pull_request.types import PullRequestWithFiles -from sentry.shared_integrations.exceptions import ApiError - -logger = logging.getLogger(__name__) - - -@cell_silo_endpoint -class OrganizationPullRequestDetailsEndpoint(OrganizationEndpoint): - owner = ApiOwner.EMERGE_TOOLS - publish_status = { - "GET": ApiPublishStatus.EXPERIMENTAL, - } - - def get( - self, request: Request, organization: Organization, repo_name: str, pr_number: str - ) -> Response: - """ - Get files changed in a pull request and general information about the pull request. - Returns normalized data that works across GitHub, GitLab, and Bitbucket. - """ - analytics.record( - PreprodApiPrPageDetailsEvent( - organization_id=organization.id, - user_id=request.user.id, - repo_name=repo_name, - pr_number=pr_number, - ) - ) - - if not features.has("organizations:pr-page", organization, actor=request.user): - return Response({"error": "Feature not enabled"}, status=403) - - client = get_github_client(organization, repo_name) - if not client: - logger.warning( - "No GitHub client found for organization", - extra={"organization_id": organization.id}, - ) - error_data = PullRequestDataAdapter.create_error_response( - error="integration_not_found", - message="No GitHub integration found for this repository", - details="Unable to find a GitHub integration for the specified repository", - ) - return Response(error_data.dict(), status=404) - - try: - # TODO(telkins): handle pagination - pr_files = client.get_pull_request_files(repo_name, pr_number) - # TODO(telkins): push this into client - pr_details = client.get(f"/repos/{repo_name}/pulls/{pr_number}") - - logger.info( - "Fetched PR data from GitHub", - extra={ - "organization_id": organization.id, - "pr_number": pr_number, - "file_count": len(pr_files) if pr_files else 0, - }, - ) - - normalized_data: PullRequestWithFiles = PullRequestDataAdapter.from_github_pr_data( - pr_details, pr_files or [], organization.id - ) - - return Response(normalized_data.dict(), status=200) - - except ApiError: - logger.exception( - "GitHub API error when fetching PR data", - extra={ - "organization_id": organization.id, - "pr_number": pr_number, - }, - ) - error_data = PullRequestDataAdapter.create_error_response( - error="api_error", - message="Failed to fetch pull request data from GitHub", - details="A problem occurred when communicating with GitHub. Please try again later.", - ) - return Response(error_data.dict(), status=502) - except Exception: - logger.exception( - "Unexpected error fetching PR data", - extra={ - "organization_id": organization.id, - "pr_number": pr_number, - }, - ) - error_data = PullRequestDataAdapter.create_error_response( - error="internal_error", - message="An unexpected error occurred while fetching pull request data", - ) - return Response(error_data.dict(), status=500) diff --git a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_size_analysis_download.py b/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_size_analysis_download.py deleted file mode 100644 index d4189301992438..00000000000000 --- a/src/sentry/preprod/api/endpoints/pull_request/organization_pullrequest_size_analysis_download.py +++ /dev/null @@ -1,75 +0,0 @@ -from __future__ import annotations - -from django.http.response import HttpResponseBase -from rest_framework.request import Request -from rest_framework.response import Response - -from sentry import analytics, features -from sentry.api.api_owners import ApiOwner -from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import OrganizationEndpoint -from sentry.models.organization import Organization -from sentry.preprod.analytics import PreprodApiPrPageSizeAnalysisDownloadEvent -from sentry.preprod.api.bases.preprod_artifact_endpoint import PreprodArtifactResourceDoesNotExist -from sentry.preprod.models import PreprodArtifact -from sentry.preprod.quotas import get_size_retention_cutoff -from sentry.preprod.size_analysis.download import ( - SizeAnalysisError, - get_size_analysis_error_response, - get_size_analysis_response, -) - - -@cell_silo_endpoint -class OrganizationPullRequestSizeAnalysisDownloadEndpoint(OrganizationEndpoint): - owner = ApiOwner.EMERGE_TOOLS - publish_status = { - "GET": ApiPublishStatus.EXPERIMENTAL, - } - - def get( - self, request: Request, organization: Organization, artifact_id: str - ) -> HttpResponseBase: - """ - Download size analysis results for a preprod artifact - ```````````````````````````````````````````````````` - - Download the size analysis results for a preprod artifact. This is separate from the - ProjectPreprodArtifactSizeAnalysisDownloadEndpoint as PR page is not tied to a project. - - :pparam string organization_id_or_slug: the id or slug of the organization the - artifact belongs to. - :pparam string artifact_id: the ID of the preprod artifact to download size analysis for. - :auth: required - """ - - analytics.record( - PreprodApiPrPageSizeAnalysisDownloadEvent( - organization_id=organization.id, - user_id=request.user.id, - artifact_id=artifact_id, - ) - ) - - if not features.has("organizations:pr-page", organization, actor=request.user): - return Response({"detail": "Feature not enabled"}, status=403) - - try: - artifact = PreprodArtifact.objects.get( - id=int(artifact_id), - project__organization_id=organization.id, - ) - except (PreprodArtifact.DoesNotExist, ValueError): - raise PreprodArtifactResourceDoesNotExist - - cutoff = get_size_retention_cutoff(organization) - if artifact.date_added < cutoff: - return Response({"detail": "This build's size data has expired."}, status=404) - - all_size_metrics = list(artifact.get_size_metrics()) - - try: - return get_size_analysis_response(all_size_metrics) - except SizeAnalysisError as e: - return get_size_analysis_error_response(e) diff --git a/src/sentry/preprod/api/endpoints/urls.py b/src/sentry/preprod/api/endpoints/urls.py index ac6ba3814e1126..351bcdfe02ad5c 100644 --- a/src/sentry/preprod/api/endpoints/urls.py +++ b/src/sentry/preprod/api/endpoints/urls.py @@ -57,11 +57,6 @@ from .public.project_preprod_snapshot_status_check_rules import ( ProjectPreprodSnapshotStatusCheckRulesEndpoint, ) -from .pull_request.organization_pullrequest_comments import OrganizationPrCommentsEndpoint -from .pull_request.organization_pullrequest_details import OrganizationPullRequestDetailsEndpoint -from .pull_request.organization_pullrequest_size_analysis_download import ( - OrganizationPullRequestSizeAnalysisDownloadEndpoint, -) from .snapshots.preprod_artifact_snapshot import ( OrganizationPreprodSnapshotEndpoint, ProjectPreprodSnapshotEndpoint, @@ -183,22 +178,6 @@ PreprodArtifactRerunStatusChecksEndpoint.as_view(), name="sentry-api-0-organization-preprod-artifact-rerun-status-checks", ), - # PR page - re_path( - r"^(?P[^/]+)/pullrequest-details/(?P.+?)/(?P\d+)/$", - OrganizationPullRequestDetailsEndpoint.as_view(), - name="sentry-api-0-organization-pullrequest-details", - ), - re_path( - r"^(?P[^/]+)/pull-requests/size-analysis/(?P[^/]+)/$", - OrganizationPullRequestSizeAnalysisDownloadEndpoint.as_view(), - name="sentry-api-0-organization-pullrequest-size-analysis-download", - ), - re_path( - r"^(?P[^/]+)/pr-comments/(?P.+?)/(?P\d+)/$", - OrganizationPrCommentsEndpoint.as_view(), - name="sentry-api-0-organization-pr-comments", - ), re_path( r"^(?P[^/]+)/builds/$", BuildsEndpoint.as_view(), diff --git a/static/app/router/routes.tsx b/static/app/router/routes.tsx index 159a1a90de0f85..c629be99c4edc9 100644 --- a/static/app/router/routes.tsx +++ b/static/app/router/routes.tsx @@ -2402,22 +2402,6 @@ function buildRoutes(): RouteObject[] { children: preprodChildren, }; - const pullRequestChildren: SentryRouteObject[] = [ - { - path: ':repoOrg/:repoName/:prId/', - component: make( - () => import('sentry/views/pullRequest/details/pullRequestDetails') - ), - }, - ]; - - const pullRequestRoutes: SentryRouteObject = { - path: '/pull/', - component: make(() => import('sentry/views/pullRequest/index')), - withOrgPath: true, - children: pullRequestChildren, - }; - const feedbackV2Children: SentryRouteObject[] = [ { index: true, @@ -2789,7 +2773,6 @@ function buildRoutes(): RouteObject[] { alertRoutes, monitorRoutes, preprodRoutes, - pullRequestRoutes, replayRoutes, releasesRoutes, statsRoutes, diff --git a/static/app/utils/api/knownSentryApiUrls.generated.ts b/static/app/utils/api/knownSentryApiUrls.generated.ts index dfc94b8bcff807..7c2bae07d8aa88 100644 --- a/static/app/utils/api/knownSentryApiUrls.generated.ts +++ b/static/app/utils/api/knownSentryApiUrls.generated.ts @@ -470,7 +470,6 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/plugins/' | '/organizations/$organizationIdOrSlug/plugins/$pluginSlug/deprecation-info/' | '/organizations/$organizationIdOrSlug/plugins/configs/' - | '/organizations/$organizationIdOrSlug/pr-comments/$repoName/$prNumber/' | '/organizations/$organizationIdOrSlug/preprod-artifact/rerun-analysis/$headArtifactId/' | '/organizations/$organizationIdOrSlug/preprod-artifact/rerun-status-checks/$headArtifactId/' | '/organizations/$organizationIdOrSlug/preprod/quota/' @@ -508,8 +507,6 @@ export type KnownSentryApiUrls = | '/organizations/$organizationIdOrSlug/projects/' | '/organizations/$organizationIdOrSlug/projects/$projectIdOrSlug/detectors/' | '/organizations/$organizationIdOrSlug/prompts-activity/' - | '/organizations/$organizationIdOrSlug/pull-requests/size-analysis/$artifactId/' - | '/organizations/$organizationIdOrSlug/pullrequest-details/$repoName/$prNumber/' | '/organizations/$organizationIdOrSlug/recent-searches/' | '/organizations/$organizationIdOrSlug/related-issues/' | '/organizations/$organizationIdOrSlug/relay_usage/' diff --git a/static/app/views/pullRequest/details/header/pullRequestDetailsHeaderContent.tsx b/static/app/views/pullRequest/details/header/pullRequestDetailsHeaderContent.tsx deleted file mode 100644 index 843f282d7ae306..00000000000000 --- a/static/app/views/pullRequest/details/header/pullRequestDetailsHeaderContent.tsx +++ /dev/null @@ -1,113 +0,0 @@ -import styled from '@emotion/styled'; - -import {Tag} from '@sentry/scraps/badge'; -import {LinkButton} from '@sentry/scraps/button'; -import {Container, Flex, Stack} from '@sentry/scraps/layout'; -import {Heading, Text} from '@sentry/scraps/text'; - -import {IconBranch, IconCommit, IconFile, IconGithub} from 'sentry/icons'; -import {t, tn} from 'sentry/locale'; -import type { - PullRequestDetailsSuccessResponse, - PullRequestState, -} from 'sentry/views/pullRequest/types/pullRequestDetailsTypes'; - -interface PullRequestDetailsHeaderContentProps { - pullRequest: PullRequestDetailsSuccessResponse; -} - -export function PullRequestDetailsHeaderContent({ - pullRequest, -}: PullRequestDetailsHeaderContentProps) { - const {pull_request: pr} = pullRequest; - - return ( - - - - - #{pr.number}: {pr.title || 'Untitled'} - - - {getPrStateBadge(pr.state)} - - {pr.author.avatar_url && } - {pr.author.username && ( - - {pr.author.username} - - )} - - - - - {pr.source_branch} - - - - - +{pr.additions} - - - / - - - -{pr.deletions} - - - - - - {tn('%s commit', '%s commits', pr.commits_count)} - - - - - - {tn('%s file', '%s files', pr.changed_files_count)} - - - - - {pr.url && ( - } href={pr.url} external> - {t('View on GitHub')} - - )} - - - {pr.description && ( - - {pr.description} - - )} - - ); -} - -function getPrStateBadge(state: PullRequestState): React.ReactNode | null { - switch (state) { - case 'open': - return Open; - case 'closed': - return Closed; - case 'merged': - return Merged; - case 'draft': - return Draft; - default: - return null; - } -} - -const GitHubAvatar = styled('img')` - width: 20px; - height: 20px; - border-radius: 50%; -`; - -const BranchLabel = styled(Text)` - padding: ${p => p.theme.space.xs} ${p => p.theme.space.sm}; - background-color: ${p => p.theme.colors.gray100}; - border-radius: ${p => p.theme.radius.md}; -`; diff --git a/static/app/views/pullRequest/details/main/prFilesList.tsx b/static/app/views/pullRequest/details/main/prFilesList.tsx deleted file mode 100644 index c0b7b015f2988e..00000000000000 --- a/static/app/views/pullRequest/details/main/prFilesList.tsx +++ /dev/null @@ -1,283 +0,0 @@ -import {Fragment, useEffect, useMemo, useState} from 'react'; -import styled from '@emotion/styled'; - -import {Container, Flex} from '@sentry/scraps/layout'; -import {Heading, Text} from '@sentry/scraps/text'; - -import {IconChevron} from 'sentry/icons'; -import type {PullRequestDetailsSuccessResponse} from 'sentry/views/pullRequest/types/pullRequestDetailsTypes'; - -type PRFileData = PullRequestDetailsSuccessResponse['files'][number]; - -interface ParsedDiffLine { - content: string; - type: 'addition' | 'deletion' | 'context' | 'header'; - newLineNumber?: number; - oldLineNumber?: number; -} - -function parsePatch(patch: string): ParsedDiffLine[] { - const lines = patch.split('\n'); - const parsedLines: ParsedDiffLine[] = []; - let oldLineNumber = 0; - let newLineNumber = 0; - - for (const line of lines) { - if (line.startsWith('@@')) { - const match = line.match(/@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/); - if (match?.[1] && match[2]) { - oldLineNumber = parseInt(match[1], 10); - newLineNumber = parseInt(match[2], 10); - } - parsedLines.push({ - content: line, - type: 'header', - }); - } else if (line.startsWith('+')) { - parsedLines.push({ - content: line.slice(1), // Remove leading + - type: 'addition', - newLineNumber: newLineNumber++, - }); - } else if (line.startsWith('-')) { - parsedLines.push({ - content: line.slice(1), // Remove leading - - type: 'deletion', - oldLineNumber: oldLineNumber++, - }); - } else { - parsedLines.push({ - content: line.startsWith(' ') ? line.slice(1) : line, // Remove leading space for context lines - type: 'context', - oldLineNumber: oldLineNumber++, - newLineNumber: newLineNumber++, - }); - } - } - - return parsedLines; -} - -interface PRFilesListProps { - files: PRFileData[]; -} - -export function PRFilesList({files}: PRFilesListProps) { - const [expandedFiles, setExpandedFiles] = useState>({}); - - // Use filename as key instead of index for more stable state - useEffect(() => { - const initialExpanded: Record = {}; - files.forEach(file => { - initialExpanded[file.filename] = true; - }); - setExpandedFiles(initialExpanded); - }, [files]); - - const toggleFileExpanded = (filename: string) => { - setExpandedFiles(prev => ({ - ...prev, - [filename]: !prev[filename], - })); - }; - - const totalStats = useMemo(() => { - return files.reduce( - (acc, file) => ({ - additions: acc.additions + file.additions, - deletions: acc.deletions + file.deletions, - }), - {additions: 0, deletions: 0} - ); - }, [files]); - - if (!files.length) { - return null; - } - - return ( - - - - Files ({files.length}) - - - +{totalStats.additions} -{totalStats.deletions} - - - {files.map(file => { - const isExpanded = expandedFiles[file.filename] ?? true; - - return ( - - toggleFileExpanded(file.filename)} - > - - - - {file.filename} - - - - - +{file.additions} - - - -{file.deletions} - - - ({file.status}) - - - - {isExpanded && file.patch && ( - - - - {parsePatch(file.patch).map((line, lineIndex) => { - return ( - - - - {line.oldLineNumber || ''} - - - {line.newLineNumber || ''} - - - {line.content} - - - - ); - })} - - - - )} - - ); - })} - - ); -} - -const CollapseIcon = styled(IconChevron)` - color: ${p => p.theme.colors.gray400}; - transition: transform 0.2s ease; -`; - -const DiffTable = styled('table')` - width: 100%; - border-collapse: collapse; - font-size: 11px; - font-family: ${p => p.theme.font.family.mono}; - margin: 0; -`; - -const DiffRow = styled('tr')` - &.addition { - background-color: #e6ffec; - - td { - color: ${p => p.theme.colors.gray800}; - - &:not(.old-line-number):not(.new-line-number) { - background-color: #d1f4db; - border-left: 3px solid #28a745; - } - } - - .old-line-number { - background-color: ${p => p.theme.tokens.background.primary}; - color: ${p => p.theme.tokens.content.secondary}; - } - .new-line-number { - background-color: #d1f4db; - color: ${p => p.theme.tokens.content.secondary}; - } - } - - &.deletion { - background-color: #ffebe9; - - td { - color: ${p => p.theme.colors.gray800}; - - &:not(.old-line-number):not(.new-line-number) { - background-color: #ffd7d5; - border-left: 3px solid #d73a49; - } - } - - .old-line-number, - .new-line-number { - background-color: #ffd7d5; - color: ${p => p.theme.tokens.content.secondary}; - } - } - - &.context { - background-color: ${p => p.theme.tokens.background.primary}; - - td { - color: ${p => p.theme.colors.gray500}; - } - } - - &.header { - background-color: ${p => p.theme.tokens.background.secondary}; - - td { - color: ${p => p.theme.tokens.content.accent}; - font-weight: ${p => p.theme.font.weight.sans.medium}; - } - } -`; - -const LineNumber = styled('td')` - width: 50px; - padding: ${p => p.theme.space.xs} ${p => p.theme.space.md}; - text-align: right; - border-right: 1px solid ${p => p.theme.tokens.border.primary}; - background-color: ${p => p.theme.tokens.background.primary}; - color: ${p => p.theme.tokens.content.secondary}; - font-size: 10px; - user-select: none; - vertical-align: top; - - &.old-line-number { - border-right: 1px solid ${p => p.theme.tokens.border.primary}; - } - - &.new-line-number { - border-right: 2px solid ${p => p.theme.tokens.border.primary}; - } -`; - -const DiffContent = styled('td')` - padding: ${p => p.theme.space.xs} ${p => p.theme.space.md}; - white-space: pre; - line-height: 1.4; - vertical-align: top; - - code { - background: none; - padding: 0; - font-family: inherit; - font-size: inherit; - } -`; diff --git a/static/app/views/pullRequest/details/main/pullRequestDetailsMainContent.tsx b/static/app/views/pullRequest/details/main/pullRequestDetailsMainContent.tsx deleted file mode 100644 index c4031debbbfef7..00000000000000 --- a/static/app/views/pullRequest/details/main/pullRequestDetailsMainContent.tsx +++ /dev/null @@ -1,19 +0,0 @@ -import {Stack} from '@sentry/scraps/layout'; - -import type {PullRequestDetailsSuccessResponse} from 'sentry/views/pullRequest/types/pullRequestDetailsTypes'; - -import {PRFilesList} from './prFilesList'; - -interface PullRequestDetailsMainContentProps { - pullRequest: PullRequestDetailsSuccessResponse; -} - -export function PullRequestDetailsMainContent({ - pullRequest, -}: PullRequestDetailsMainContentProps) { - return ( - - - - ); -} diff --git a/static/app/views/pullRequest/details/main/pullRequestDetailsSizeContent.tsx b/static/app/views/pullRequest/details/main/pullRequestDetailsSizeContent.tsx deleted file mode 100644 index d827eae041458e..00000000000000 --- a/static/app/views/pullRequest/details/main/pullRequestDetailsSizeContent.tsx +++ /dev/null @@ -1,105 +0,0 @@ -import {useState} from 'react'; - -import {CompactSelect} from '@sentry/scraps/compactSelect'; -import {Flex, Grid, Stack, Container} from '@sentry/scraps/layout'; -import {Heading} from '@sentry/scraps/text'; - -import {t} from 'sentry/locale'; -import {useApiQuery} from 'sentry/utils/queryClient'; -import {useOrganization} from 'sentry/utils/useOrganization'; -import {BuildDetailsMainContent} from 'sentry/views/preprod/buildDetails/main/buildDetailsMainContent'; -import {BuildDetailsSidebarContent} from 'sentry/views/preprod/buildDetails/sidebar/buildDetailsSidebarContent'; -import type {AppSizeApiResponse} from 'sentry/views/preprod/types/appSizeTypes'; -import type {BuildDetailsApiResponse} from 'sentry/views/preprod/types/buildDetailsTypes'; - -interface PullRequestDetailsSizeContentProps { - buildDetails: BuildDetailsApiResponse[]; -} - -export function PullRequestDetailsSizeContent({ - buildDetails, -}: PullRequestDetailsSizeContentProps) { - const organization = useOrganization(); - const [selectedBuildId, setSelectedBuildId] = useState(buildDetails[0]?.id); - - const appSizeQuery = useApiQuery( - // @ts-expect-error TODO(ryan953): Invalid useApiQuery path (should be organization prefix?) - [`/projects/${organization.slug}/pull-requests/size-analysis/${selectedBuildId}/`], - { - staleTime: 0, - enabled: !!selectedBuildId, - } - ); - - if (buildDetails.length === 0 || !selectedBuildId) { - return
No build details found
; - } - - const getBuildDetailsLabel = (buildDetail: BuildDetailsApiResponse) => { - const {id, app_info} = buildDetail; - let label = `#${id}`; - - if (app_info?.app_id !== null) { - label += ` ${app_info.app_id}`; - } - if (app_info?.version !== null) { - label += ` v${app_info.version}`; - } - if (app_info?.build_number !== null) { - label += ` (${app_info.build_number})`; - } - if (app_info?.build_configuration !== null) { - label += ` ${app_info.build_configuration}`; - } - return label; - }; - - const selectOptions = buildDetails.map(buildDetail => ({ - label: getBuildDetailsLabel(buildDetail), - value: buildDetail.id.toString(), - buildDetail, - })); - const selectedBuildDetail = buildDetails.find( - buildDetail => buildDetail.id === selectedBuildId - ); - - return ( - - {buildDetails.length > 1 && ( - - {t('Builds (%s)', buildDetails.length)} - - { - setSelectedBuildId(option?.value); - }} - options={selectOptions} - aria-label={t('Select build')} - /> - - - )} - - - {buildDetails.length > 0 && ( - - )} - - - {}} - /> - - - - ); -} diff --git a/static/app/views/pullRequest/details/pullRequestDetails.tsx b/static/app/views/pullRequest/details/pullRequestDetails.tsx deleted file mode 100644 index 988ff350f656e3..00000000000000 --- a/static/app/views/pullRequest/details/pullRequestDetails.tsx +++ /dev/null @@ -1,131 +0,0 @@ -import {parseAsStringLiteral, useQueryState} from 'nuqs'; - -import {Container, Flex, Stack} from '@sentry/scraps/layout'; -import {TabList} from '@sentry/scraps/tabs'; -import {Heading, Text} from '@sentry/scraps/text'; - -import * as Layout from 'sentry/components/layouts/thirds'; -import {LoadingIndicator} from 'sentry/components/loadingIndicator'; -import {t} from 'sentry/locale'; -import {getApiUrl} from 'sentry/utils/api/getApiUrl'; -import {useApiQuery} from 'sentry/utils/queryClient'; -import {useOrganization} from 'sentry/utils/useOrganization'; -import {useParams} from 'sentry/utils/useParams'; -import {PullRequestDetailsHeaderContent} from 'sentry/views/pullRequest/details/header/pullRequestDetailsHeaderContent'; -import {PullRequestDetailsMainContent} from 'sentry/views/pullRequest/details/main/pullRequestDetailsMainContent'; -import {PullRequestDetailsSizeContent} from 'sentry/views/pullRequest/details/main/pullRequestDetailsSizeContent'; -import type { - PullRequestDetailsErrorResponse, - PullRequestDetailsResponse, - PullRequestDetailsSuccessResponse, -} from 'sentry/views/pullRequest/types/pullRequestDetailsTypes'; - -export default function PullRequestDetails() { - const organization = useOrganization(); - const params = useParams() as {prId: string; repoName: string; repoOrg: string}; - const [selectedTab, setSelectedTab] = useQueryState( - 'tab', - parseAsStringLiteral(['files', 'size_analysis'] as const).withDefault('files') - ); - - const pullRequestQuery = useApiQuery( - [ - getApiUrl( - '/organizations/$organizationIdOrSlug/pullrequest-details/$repoName/$prNumber/', - { - path: { - organizationIdOrSlug: organization.slug, - repoName: params.repoName, - prNumber: params.prId, - }, - } - ), - ], - { - staleTime: 0, - enabled: !!params.repoName && !!params.prId, - } - ); - - const {data, isLoading, error} = pullRequestQuery; - - if (isLoading) { - return ( - - - Pull Request Details - - - - - - - - - - ); - } - - const errorData = error?.responseJSON as PullRequestDetailsErrorResponse | undefined; - if (error || !data) { - return ( - - - Pull Request Details - - - - - - Error Loading Pull Request - - {errorData?.message || - error?.message || - 'Failed to load pull request details'} - - {errorData?.details && ( - - {errorData.details} - - )} - - - - - - ); - } - - const prSuccessData = data as PullRequestDetailsSuccessResponse; - const hasSizeAnalysis = prSuccessData.build_details.length > 0; - - let mainContent = ; - if (selectedTab === 'size_analysis' && hasSizeAnalysis) { - mainContent = ( - - ); - } - - return ( - - - - - - {t('Files')} - {hasSizeAnalysis ? ( - {t('Size Analysis')} - ) : null} - - - - - - {mainContent} - - - ); -} diff --git a/static/app/views/pullRequest/index.tsx b/static/app/views/pullRequest/index.tsx deleted file mode 100644 index 17f25ca454d18c..00000000000000 --- a/static/app/views/pullRequest/index.tsx +++ /dev/null @@ -1,36 +0,0 @@ -import {Outlet} from 'react-router-dom'; - -import {Alert} from '@sentry/scraps/alert'; -import {Stack} from '@sentry/scraps/layout'; - -import Feature from 'sentry/components/acl/feature'; -import {NoProjectMessage} from 'sentry/components/noProjectMessage'; -import {t} from 'sentry/locale'; -import {UrlParamBatchProvider} from 'sentry/utils/url/urlParamBatchContext'; -import {useOrganization} from 'sentry/utils/useOrganization'; - -export default function PullRequestContainer() { - const organization = useOrganization(); - - return ( - ( - - - - {t("You don't have access to this feature")} - - - - )} - > - - - - - - - ); -} diff --git a/static/app/views/pullRequest/types/pullRequestDetailsTypes.ts b/static/app/views/pullRequest/types/pullRequestDetailsTypes.ts deleted file mode 100644 index 119832ac453dd7..00000000000000 --- a/static/app/views/pullRequest/types/pullRequestDetailsTypes.ts +++ /dev/null @@ -1,58 +0,0 @@ -import type {BuildDetailsApiResponse} from 'sentry/views/preprod/types/buildDetailsTypes'; - -export type PullRequestState = 'open' | 'closed' | 'merged' | 'draft'; -type PullRequestFileStatus = 'added' | 'modified' | 'removed' | 'renamed'; - -interface PullRequestAuthor { - avatar_url: string | null; - display_name: string | null; - id: string | null; - username: string | null; -} - -interface PullRequestDetails { - additions: number; - author: PullRequestAuthor; - changed_files_count: number; - closed_at: string | null; - commits_count: number; - created_at: string | null; - deletions: number; - description: string | null; - id: string | null; - merged_at: string | null; - number: number; - source_branch: string | null; - state: PullRequestState; - target_branch: string | null; - title: string | null; - updated_at: string | null; - url: string | null; -} - -interface PullRequestFileChange { - additions: number; - changes: number; - deletions: number; - filename: string; - patch: string | null; - previous_filename: string | null; - sha: string | null; - status: PullRequestFileStatus; -} - -export interface PullRequestDetailsSuccessResponse { - build_details: BuildDetailsApiResponse[]; - files: PullRequestFileChange[]; - pull_request: PullRequestDetails; -} - -export interface PullRequestDetailsErrorResponse { - error: string; - message: string; - details?: string; -} - -export type PullRequestDetailsResponse = - | PullRequestDetailsSuccessResponse - | PullRequestDetailsErrorResponse; diff --git a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_comments.py b/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_comments.py deleted file mode 100644 index b93095258785a9..00000000000000 --- a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_comments.py +++ /dev/null @@ -1,361 +0,0 @@ -from unittest.mock import patch - -from rest_framework.test import APIRequestFactory - -from sentry.integrations.github.client import GitHubApiRequestType -from sentry.models.repository import Repository -from sentry.preprod.api.endpoints.pull_request.organization_pullrequest_comments import ( - OrganizationPrCommentsEndpoint, -) -from sentry.shared_integrations.exceptions import ApiError -from sentry.testutils.cases import TestCase -from sentry.testutils.helpers.features import with_feature - - -class OrganizationPrCommentsEndpointTest(TestCase): - def setUp(self) -> None: - super().setUp() - self.factory = APIRequestFactory() - - self.integration = self.create_integration( - organization=self.organization, - provider="github", - name="Test GitHub Integration", - external_id="12345", - metadata={ - "access_token": "test-token", - "expires_at": None, - "installation": {"id": 12345, "account": {"login": "getsentry"}}, - }, - ) - - self.repository = Repository.objects.create( - organization_id=self.organization.id, - name="getsentry/sentry", - provider="integrations:github", - integration_id=self.integration.id, - ) - - self.mock_general_comments = [ - { - "id": 1, - "node_id": "IC_test1", - "url": "https://api.github.com/repos/getsentry/sentry/issues/comments/1", - "html_url": "https://github.com/getsentry/sentry/pull/100#issuecomment-1", - "body": "This looks great!", - "user": { - "login": "testuser1", - "id": 123, - "node_id": "U_test1", - "avatar_url": "https://avatars.githubusercontent.com/u/123", - "gravatar_id": "", - "url": "https://api.github.com/users/testuser1", - "html_url": "https://github.com/testuser1", - "followers_url": "https://api.github.com/users/testuser1/followers", - "following_url": "https://api.github.com/users/testuser1/following{/other_user}", - "gists_url": "https://api.github.com/users/testuser1/gists{/gist_id}", - "starred_url": "https://api.github.com/users/testuser1/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/testuser1/subscriptions", - "organizations_url": "https://api.github.com/users/testuser1/orgs", - "repos_url": "https://api.github.com/users/testuser1/repos", - "events_url": "https://api.github.com/users/testuser1/events{/privacy}", - "received_events_url": "https://api.github.com/users/testuser1/received_events", - "type": "User", - "site_admin": False, - }, - "created_at": "2023-01-01T12:00:00Z", - "updated_at": "2023-01-01T12:00:00Z", - "issue_url": "https://api.github.com/repos/getsentry/sentry/issues/100", - "author_association": "CONTRIBUTOR", - }, - { - "id": 2, - "node_id": "IC_test2", - "url": "https://api.github.com/repos/getsentry/sentry/issues/comments/2", - "html_url": "https://github.com/getsentry/sentry/pull/100#issuecomment-2", - "body": "Can you add tests?", - "user": { - "login": "testuser2", - "id": 456, - "node_id": "U_test2", - "avatar_url": "https://avatars.githubusercontent.com/u/456", - "gravatar_id": "", - "url": "https://api.github.com/users/testuser2", - "html_url": "https://github.com/testuser2", - "followers_url": "https://api.github.com/users/testuser2/followers", - "following_url": "https://api.github.com/users/testuser2/following{/other_user}", - "gists_url": "https://api.github.com/users/testuser2/gists{/gist_id}", - "starred_url": "https://api.github.com/users/testuser2/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/testuser2/subscriptions", - "organizations_url": "https://api.github.com/users/testuser2/orgs", - "repos_url": "https://api.github.com/users/testuser2/repos", - "events_url": "https://api.github.com/users/testuser2/events{/privacy}", - "received_events_url": "https://api.github.com/users/testuser2/received_events", - "type": "User", - "site_admin": False, - }, - "created_at": "2023-01-02T10:30:00Z", - "updated_at": "2023-01-02T10:30:00Z", - "issue_url": "https://api.github.com/repos/getsentry/sentry/issues/100", - "author_association": "MEMBER", - }, - ] - - self.mock_review_comments = [ - { - "id": 10, - "node_id": "RC_test10", - "url": "https://api.github.com/repos/getsentry/sentry/pulls/comments/10", - "html_url": "https://github.com/getsentry/sentry/pull/100#discussion_r10", - "path": "src/components/Button.tsx", - "line": 25, - "body": "Consider using a const here", - "user": { - "login": "reviewer1", - "id": 789, - "node_id": "U_test789", - "avatar_url": "https://avatars.githubusercontent.com/u/789", - "gravatar_id": "", - "url": "https://api.github.com/users/reviewer1", - "html_url": "https://github.com/reviewer1", - "followers_url": "https://api.github.com/users/reviewer1/followers", - "following_url": "https://api.github.com/users/reviewer1/following{/other_user}", - "gists_url": "https://api.github.com/users/reviewer1/gists{/gist_id}", - "starred_url": "https://api.github.com/users/reviewer1/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/reviewer1/subscriptions", - "organizations_url": "https://api.github.com/users/reviewer1/orgs", - "repos_url": "https://api.github.com/users/reviewer1/repos", - "events_url": "https://api.github.com/users/reviewer1/events{/privacy}", - "received_events_url": "https://api.github.com/users/reviewer1/received_events", - "type": "User", - "site_admin": False, - }, - "created_at": "2023-01-01T14:00:00Z", - "updated_at": "2023-01-01T14:00:00Z", - "author_association": "MEMBER", - "commit_id": "abc123def456789", - "original_commit_id": "abc123def456789", - "diff_hunk": "@@ -20,6 +20,8 @@ function Button() {", - "pull_request_url": "https://api.github.com/repos/getsentry/sentry/pulls/100", - "pull_request_review_id": 1, - "_links": { - "self": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/comments/10" - }, - "html": {"href": "https://github.com/getsentry/sentry/pull/100#discussion_r10"}, - "pull_request": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/100" - }, - }, - }, - { - "id": 11, - "node_id": "RC_test11", - "url": "https://api.github.com/repos/getsentry/sentry/pulls/comments/11", - "html_url": "https://github.com/getsentry/sentry/pull/100#discussion_r11", - "path": "src/components/Button.tsx", - "line": 30, - "body": "Good catch!", - "user": { - "login": "reviewer2", - "id": 101, - "node_id": "U_test101", - "avatar_url": "https://avatars.githubusercontent.com/u/101", - "gravatar_id": "", - "url": "https://api.github.com/users/reviewer2", - "html_url": "https://github.com/reviewer2", - "followers_url": "https://api.github.com/users/reviewer2/followers", - "following_url": "https://api.github.com/users/reviewer2/following{/other_user}", - "gists_url": "https://api.github.com/users/reviewer2/gists{/gist_id}", - "starred_url": "https://api.github.com/users/reviewer2/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/reviewer2/subscriptions", - "organizations_url": "https://api.github.com/users/reviewer2/orgs", - "repos_url": "https://api.github.com/users/reviewer2/repos", - "events_url": "https://api.github.com/users/reviewer2/events{/privacy}", - "received_events_url": "https://api.github.com/users/reviewer2/received_events", - "type": "User", - "site_admin": False, - }, - "created_at": "2023-01-01T15:00:00Z", - "updated_at": "2023-01-01T15:00:00Z", - "author_association": "CONTRIBUTOR", - "commit_id": "abc123def456789", - "original_commit_id": "abc123def456789", - "diff_hunk": "@@ -25,6 +25,8 @@ function Button() {", - "pull_request_url": "https://api.github.com/repos/getsentry/sentry/pulls/100", - "pull_request_review_id": 1, - "_links": { - "self": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/comments/11" - }, - "html": {"href": "https://github.com/getsentry/sentry/pull/100#discussion_r11"}, - "pull_request": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/100" - }, - }, - }, - { - "id": 12, - "node_id": "RC_test12", - "url": "https://api.github.com/repos/getsentry/sentry/pulls/comments/12", - "html_url": "https://github.com/getsentry/sentry/pull/100#discussion_r12", - "path": "src/utils/helper.ts", - "line": 10, - "body": "This could be simplified", - "user": { - "login": "reviewer1", - "id": 789, - "node_id": "U_test789", - "avatar_url": "https://avatars.githubusercontent.com/u/789", - "gravatar_id": "", - "url": "https://api.github.com/users/reviewer1", - "html_url": "https://github.com/reviewer1", - "followers_url": "https://api.github.com/users/reviewer1/followers", - "following_url": "https://api.github.com/users/reviewer1/following{/other_user}", - "gists_url": "https://api.github.com/users/reviewer1/gists{/gist_id}", - "starred_url": "https://api.github.com/users/reviewer1/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/reviewer1/subscriptions", - "organizations_url": "https://api.github.com/users/reviewer1/orgs", - "repos_url": "https://api.github.com/users/reviewer1/repos", - "events_url": "https://api.github.com/users/reviewer1/events{/privacy}", - "received_events_url": "https://api.github.com/users/reviewer1/received_events", - "type": "User", - "site_admin": False, - }, - "created_at": "2023-01-02T09:00:00Z", - "updated_at": "2023-01-02T09:00:00Z", - "author_association": "MEMBER", - "commit_id": "abc123def456789", - "original_commit_id": "abc123def456789", - "diff_hunk": "@@ -8,6 +8,8 @@ function helper() {", - "pull_request_url": "https://api.github.com/repos/getsentry/sentry/pulls/100", - "pull_request_review_id": 2, - "_links": { - "self": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/comments/12" - }, - "html": {"href": "https://github.com/getsentry/sentry/pull/100#discussion_r12"}, - "pull_request": { - "href": "https://api.github.com/repos/getsentry/sentry/pulls/100" - }, - }, - }, - ] - - def _make_request(self, repo_name="getsentry/sentry", pr_number="100"): - """Helper to make API request.""" - request = self.factory.get("/") - request.user = self.user - endpoint = OrganizationPrCommentsEndpoint() - return endpoint.get( - request=request, - organization=self.organization, - repo_name=repo_name, - pr_number=pr_number, - ) - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_successful_pr_comments_fetch(self, mock_get): - """Test successful fetch of both general and review comments.""" - - def mock_get_side_effect(url, **kwargs): - if "issues" in url: - return self.mock_general_comments - elif "pulls" in url: - return self.mock_review_comments - return [] - - mock_get.side_effect = mock_get_side_effect - - response = self._make_request() - assert response.status_code == 200 - - # Verify both API calls were made - assert mock_get.call_count == 2 - mock_get.assert_any_call( - "/repos/getsentry/sentry/issues/100/comments", - api_request_type=GitHubApiRequestType.GET_ISSUE_COMMENTS, - ) - mock_get.assert_any_call( - "/repos/getsentry/sentry/pulls/100/comments", - api_request_type=GitHubApiRequestType.GET_PULL_REQUEST_COMMENTS, - ) - - # Verify response structure - assert "general_comments" in response.data - assert "file_comments" in response.data - - # Verify general comments - general_comments = response.data["general_comments"] - assert len(general_comments) == 2 - assert general_comments[0]["body"] == "This looks great!" - assert general_comments[1]["body"] == "Can you add tests?" - - # Verify file comments are organized by file - file_comments = response.data["file_comments"] - assert len(file_comments) == 2 # Two files have comments - assert "src/components/Button.tsx" in file_comments - assert "src/utils/helper.ts" in file_comments - - # Verify Button.tsx has 2 comments - button_comments = file_comments["src/components/Button.tsx"] - assert len(button_comments) == 2 - assert button_comments[0]["body"] == "Consider using a const here" - assert button_comments[1]["body"] == "Good catch!" - - # Verify helper.ts has 1 comment - helper_comments = file_comments["src/utils/helper.ts"] - assert len(helper_comments) == 1 - assert helper_comments[0]["body"] == "This could be simplified" - - @with_feature("organizations:pr-page") - def test_no_github_client(self) -> None: - """Test when no GitHub client is available (no integration set up).""" - Repository.objects.create( - organization_id=self.organization.id, - name="nonexistent/repo", - provider="integrations:github", - integration_id=None, # No integration - ) - - response = self._make_request(repo_name="nonexistent/repo") - - assert response.status_code == 404 - assert response.data["error"] == "integration_not_found" - assert "No GitHub integration found" in response.data["message"] - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_github_api_error(self, mock_get): - """Test GitHub API error handling.""" - # Simulate GitHub API error - mock_get.side_effect = ApiError("API rate limit exceeded") - - response = self._make_request() - - assert response.status_code == 502 - assert response.data["error"] == "api_error" - assert "Failed to fetch pull request comments from GitHub" in response.data["message"] - - @with_feature("organizations:pr-page") - def test_repository_not_found(self) -> None: - """Test when repository doesn't exist in the database.""" - response = self._make_request(repo_name="does-not/exist") - - assert response.status_code == 404 - assert response.data["error"] == "integration_not_found" - assert "No GitHub integration found" in response.data["message"] - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_unexpected_error(self, mock_get): - """Test handling of unexpected errors.""" - # Simulate unexpected error (not ApiError) - mock_get.side_effect = ValueError("Unexpected error") - - response = self._make_request() - - assert response.status_code == 500 - assert response.data["error"] == "internal_error" - assert "An unexpected error occurred" in response.data["message"] diff --git a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_details.py b/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_details.py deleted file mode 100644 index 72a9667a651644..00000000000000 --- a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_details.py +++ /dev/null @@ -1,268 +0,0 @@ -from unittest.mock import patch - -from rest_framework.test import APIRequestFactory - -from sentry.models.repository import Repository -from sentry.preprod.api.endpoints.pull_request.organization_pullrequest_details import ( - OrganizationPullRequestDetailsEndpoint, -) -from sentry.shared_integrations.exceptions import ApiError -from sentry.testutils.cases import TestCase -from sentry.testutils.helpers.features import with_feature - - -class OrganizationPullRequestDetailsEndpointTest(TestCase): - def setUp(self) -> None: - super().setUp() - self.factory = APIRequestFactory() - - self.integration = self.create_integration( - organization=self.organization, - provider="github", - name="Test GitHub Integration", - external_id="12345", - metadata={ - "access_token": "test-token", - "expires_at": None, - "installation": {"id": 12345, "account": {"login": "getsentry"}}, - }, - ) - - self.repository = Repository.objects.create( - organization_id=self.organization.id, - name="getsentry/sentry", - provider="integrations:github", - integration_id=self.integration.id, - ) - - self.mock_pr_details = { - "id": 123456, - "number": 100, - "title": "Add new feature", - "body": "This PR adds a new feature to improve user experience", - "state": "open", - "user": { - "id": 789, - "login": "testuser", - "name": "Test User", - "avatar_url": "https://github.com/testuser.png", - }, - "head": {"ref": "feature/new-feature"}, - "base": {"ref": "main"}, - "created_at": "2023-01-01T12:00:00Z", - "updated_at": "2023-01-02T10:30:00Z", - "merged_at": None, - "closed_at": None, - "html_url": "https://github.com/getsentry/sentry/pull/100", - "commits": 3, - "additions": 150, - "deletions": 50, - "changed_files": 5, - } - - self.mock_pr_files = [ - { - "filename": "src/components/Button.tsx", - "status": "modified", - "additions": 10, - "deletions": 2, - "changes": 12, - "sha": "abc123def456", - "patch": "@@ -1,3 +1,3 @@\n- old line\n+ new line", - }, - { - "filename": "src/utils/helper.ts", - "status": "added", - "additions": 25, - "deletions": 0, - "changes": 25, - "sha": "def456ghi789", - "patch": None, - }, - { - "filename": "tests/Button.test.tsx", - "status": "added", - "additions": 50, - "deletions": 0, - "changes": 50, - "sha": "ghi789jkl012", - "patch": None, - }, - { - "filename": "old-file.js", - "status": "removed", - "additions": 0, - "deletions": 15, - "changes": 15, - "sha": None, - "patch": None, - }, - { - "filename": "new-component.tsx", - "status": "renamed", - "additions": 5, - "deletions": 3, - "changes": 8, - "previous_filename": "old-component.tsx", - "sha": "jkl012mno345", - "patch": None, - }, - ] - - def _make_request(self, repo_name="getsentry/sentry", pr_number="100"): - """Helper to make API request.""" - request = self.factory.get("/") - request.user = self.user - endpoint = OrganizationPullRequestDetailsEndpoint() - return endpoint.get( - request=request, - organization=self.organization, - repo_name=repo_name, - pr_number=pr_number, - ) - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get_pull_request_files") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_successful_pr_details_fetch(self, mock_get, mock_get_files): - """Test successful PR details and files fetch with proper normalization.""" - # Setup GitHub API response mocks (only mock the HTTP calls) - mock_get_files.return_value = self.mock_pr_files - mock_get.return_value = self.mock_pr_details - - response = self._make_request() - assert response.status_code == 200 - - mock_get_files.assert_called_once_with("getsentry/sentry", "100") - mock_get.assert_called_once_with("/repos/getsentry/sentry/pulls/100") - - assert "pull_request" in response.data - assert "files" in response.data - - pr_data = response.data["pull_request"] - assert pr_data["id"] == "123456" - assert pr_data["number"] == 100 - assert pr_data["title"] == "Add new feature" - assert pr_data["state"] == "open" - assert pr_data["author"]["username"] == "testuser" - assert pr_data["author"]["display_name"] == "Test User" - assert pr_data["source_branch"] == "feature/new-feature" - assert pr_data["target_branch"] == "main" - assert pr_data["additions"] == 150 - assert pr_data["deletions"] == 50 - assert pr_data["changed_files_count"] == 5 - assert pr_data["created_at"] is not None - - files_data = response.data["files"] - assert len(files_data) == 5 - - modified_file = files_data[0] - assert modified_file["filename"] == "src/components/Button.tsx" - assert modified_file["status"] == "modified" - assert modified_file["additions"] == 10 - assert modified_file["deletions"] == 2 - - added_file = next(f for f in files_data if f["status"] == "added") - assert added_file["filename"] == "src/utils/helper.ts" - - renamed_file = next(f for f in files_data if f["status"] == "renamed") - assert renamed_file["previous_filename"] == "old-component.tsx" - - @with_feature("organizations:pr-page") - def test_no_github_client(self) -> None: - """Test when no GitHub client is available (no integration set up).""" - Repository.objects.create( - organization_id=self.organization.id, - name="nonexistent/repo", - provider="integrations:github", - integration_id=None, # No integration - ) - - response = self._make_request(repo_name="nonexistent/repo") - - assert response.status_code == 404 - assert response.data["error"] == "integration_not_found" - assert "No GitHub integration found" in response.data["message"] - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get_pull_request_files") - def test_github_api_error(self, mock_get_files): - """Test GitHub API error handling.""" - # Simulate GitHub API error - mock_get_files.side_effect = ApiError("API rate limit exceeded") - - response = self._make_request() - - assert response.status_code == 502 - assert response.data["error"] == "api_error" - assert "Failed to fetch pull request data from GitHub" in response.data["message"] - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get_pull_request_files") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_empty_pr_files(self, mock_get, mock_get_files): - """Test handling of PR with no files changed.""" - mock_get_files.return_value = [] - mock_get.return_value = {**self.mock_pr_details, "changed_files": 0} - - response = self._make_request() - - assert response.status_code == 200 - assert len(response.data["files"]) == 0 - assert response.data["pull_request"]["changed_files_count"] == 0 - - @with_feature("organizations:pr-page") - def test_repository_not_found(self) -> None: - """Test when repository doesn't exist in the database.""" - response = self._make_request(repo_name="does-not/exist") - - assert response.status_code == 404 - assert response.data["error"] == "integration_not_found" - assert "No GitHub integration found" in response.data["message"] - - @with_feature("organizations:pr-page") - @patch("sentry.integrations.github.client.GitHubApiClient.get_pull_request_files") - @patch("sentry.integrations.github.client.GitHubApiClient.get") - def test_missing_timestamps_handled_correctly(self, mock_get, mock_get_files): - """Test that missing timestamps are properly handled without type errors.""" - # Create PR data missing created_at and updated_at timestamps - pr_data_missing_timestamps = { - "id": 123456, - "number": 100, - "title": "Add new feature", - "body": "This PR adds a new feature to improve user experience", - "state": "open", - "user": { - "id": 789, - "login": "testuser", - "name": "Test User", - "avatar_url": "https://github.com/testuser.png", - }, - "head": {"ref": "feature/new-feature"}, - "base": {"ref": "main"}, - # Missing created_at and updated_at - "merged_at": None, - "closed_at": None, - "html_url": "https://github.com/getsentry/sentry/pull/100", - "commits": 3, - "additions": 150, - "deletions": 50, - "changed_files": 0, - } - - mock_get_files.return_value = [] - mock_get.return_value = pr_data_missing_timestamps - - response = self._make_request() - - assert response.status_code == 200 - assert "pull_request" in response.data - - pr_data = response.data["pull_request"] - # Verify that missing timestamps are handled as None - assert pr_data["created_at"] is None - assert pr_data["updated_at"] is None - # Verify other fields work correctly - assert pr_data["id"] == "123456" - assert pr_data["number"] == 100 - assert pr_data["title"] == "Add new feature" diff --git a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_size_analysis_download.py b/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_size_analysis_download.py deleted file mode 100644 index 05d06612dad9b7..00000000000000 --- a/tests/sentry/preprod/api/endpoints/pull_request/test_organization_pullrequest_size_analysis_download.py +++ /dev/null @@ -1,194 +0,0 @@ -from datetime import timedelta -from unittest.mock import patch - -from django.urls import reverse -from django.utils import timezone - -from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics -from sentry.testutils.cases import TestCase - - -class OrganizationPullRequestSizeAnalysisDownloadEndpointTest(TestCase): - def setUp(self) -> None: - super().setUp() - self.user = self.create_user() - self.organization = self.create_organization(owner=self.user) - self.project = self.create_project(organization=self.organization) - self.login_as(user=self.user) - - # Create test files - self.analysis_file = self.create_file( - name="size_analysis.json", - type="application/json", - ) - - # Create a preprod artifact - self.preprod_artifact = PreprodArtifact.objects.create( - project=self.project, - state=PreprodArtifact.ArtifactState.PROCESSED, - artifact_type=PreprodArtifact.ArtifactType.APK, - ) - - # Create size analysis metrics - self.size_metrics = PreprodArtifactSizeMetrics.objects.create( - preprod_artifact=self.preprod_artifact, - analysis_file_id=self.analysis_file.id, - metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, - identifier="main", - state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, - ) - - def _get_url(self, organization_id_or_slug=None, artifact_id=None): - org = organization_id_or_slug or self.organization.slug - artifact = artifact_id or self.preprod_artifact.id - return reverse( - "sentry-api-0-organization-pullrequest-size-analysis-download", - args=[org, artifact], - ) - - def test_size_analysis_download_success(self) -> None: - with self.feature("organizations:pr-page"): - url = self._get_url() - response = self.client.get(url) - - assert response.status_code == 200 - assert response["Content-Type"] == "application/json" - assert response["Content-Length"] == str(self.analysis_file.size) - - def test_size_analysis_download_feature_not_enabled(self) -> None: - # Test without the feature flag - url = self._get_url() - response = self.client.get(url) - - assert response.status_code == 403 - assert response.json()["detail"] == "Feature not enabled" - - def test_size_analysis_download_artifact_not_found(self) -> None: - with self.feature("organizations:pr-page"): - url = self._get_url(artifact_id=999999) - response = self.client.get(url) - - assert response.status_code == 404 - assert "The requested preprod artifact does not exist" in response.json()["detail"] - - def test_size_analysis_download_invalid_artifact_id(self) -> None: - with self.feature("organizations:pr-page"): - url = self._get_url(artifact_id="invalid-id") - response = self.client.get(url) - - assert response.status_code == 404 - assert "The requested preprod artifact does not exist" in response.json()["detail"] - - def test_size_analysis_download_artifact_from_different_organization(self) -> None: - # Create another organization and artifact - other_user = self.create_user() - other_org = self.create_organization(owner=other_user) - other_project = self.create_project(organization=other_org) - other_artifact = PreprodArtifact.objects.create( - project=other_project, - state=PreprodArtifact.ArtifactState.PROCESSED, - artifact_type=PreprodArtifact.ArtifactType.APK, - ) - - with self.feature("organizations:pr-page"): - # Try to access artifact from different org - url = self._get_url(artifact_id=other_artifact.id) - response = self.client.get(url) - - assert response.status_code == 404 - assert "The requested preprod artifact does not exist" in response.json()["detail"] - - def test_size_analysis_download_no_size_metrics(self) -> None: - # Create artifact without size metrics - artifact_without_metrics = PreprodArtifact.objects.create( - project=self.project, - state=PreprodArtifact.ArtifactState.PROCESSED, - artifact_type=PreprodArtifact.ArtifactType.APK, - ) - - with self.feature("organizations:pr-page"): - url = self._get_url(artifact_id=artifact_without_metrics.id) - response = self.client.get(url) - - assert response.status_code == 404 - assert ( - response.json()["detail"] == "Size analysis results not available for this artifact" - ) - - def test_size_analysis_download_multiple_size_metrics_same_file(self) -> None: - PreprodArtifactSizeMetrics.objects.create( - preprod_artifact=self.preprod_artifact, - analysis_file_id=self.analysis_file.id, - metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.WATCH_ARTIFACT, - identifier="watch", - state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, - ) - - with self.feature("organizations:pr-page"): - url = self._get_url() - response = self.client.get(url) - - assert response.status_code == 200 - assert response["Content-Type"] == "application/json" - - def test_size_analysis_download_multiple_different_files(self) -> None: - other_file = self.create_file( - name="other_analysis.json", - type="application/json", - ) - PreprodArtifactSizeMetrics.objects.create( - preprod_artifact=self.preprod_artifact, - analysis_file_id=other_file.id, - metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.WATCH_ARTIFACT, - identifier="watch", - state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, - ) - - with self.feature("organizations:pr-page"): - url = self._get_url() - response = self.client.get(url) - - assert response.status_code == 409 - assert ( - response.json()["detail"] - == "Multiple size analysis results found for this artifact" - ) - - def test_size_analysis_download_no_analysis_file(self) -> None: - artifact_no_file = PreprodArtifact.objects.create( - project=self.project, - state=PreprodArtifact.ArtifactState.PROCESSED, - artifact_type=PreprodArtifact.ArtifactType.APK, - ) - PreprodArtifactSizeMetrics.objects.create( - preprod_artifact=artifact_no_file, - analysis_file_id=None, - metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, - identifier="main", - state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, - ) - - with self.feature("organizations:pr-page"): - url = self._get_url(artifact_id=artifact_no_file.id) - response = self.client.get(url) - - assert response.status_code == 500 - assert ( - response.json()["detail"] == "Size analysis completed but results are unavailable" - ) - - def test_returns_404_for_expired_artifact(self) -> None: - with ( - self.feature("organizations:pr-page"), - patch( - "sentry.preprod.api.endpoints.pull_request.organization_pullrequest_size_analysis_download.get_size_retention_cutoff" - ) as mock_cutoff, - ): - mock_cutoff.return_value = timezone.now() - timedelta(days=30) - self.preprod_artifact.date_added = timezone.now() - timedelta(days=60) - self.preprod_artifact.save() - - url = self._get_url() - response = self.client.get(url) - assert response.status_code == 404 - assert response.json()["detail"] == "This build's size data has expired."