From a9be7f5607f4359cbfcf66c782365ef24b333164 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 27 Mar 2026 03:20:27 -0700 Subject: [PATCH 01/13] ref(layout): use Layout.Page on alerts (#111642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps non-compliant routes in the alerts area with Layout.Page. Part of the Layout.Page audit remediation. ## Changes - `static/app/views/alerts/list/incidents/index.tsx` — wrap `renderBody()` return and `renderDisabled()` in `Layout.Page` - `static/app/views/alerts/list/rules/alertRulesList.tsx` — wrap `PageFiltersContainer` block in `Layout.Page` - `static/app/views/alerts/create.tsx` — replace outer `Fragment` with `Layout.Page` - `static/app/views/alerts/edit.tsx` — replace outer `Fragment` with `Layout.Page` ## Affected routes | Path | Strategy | | --- | --- | | `/organizations/:orgId/alerts/` | leaf fix | | `/organizations/:orgId/alerts/rules/` | leaf fix | | `/organizations/:orgId/alerts/:projectId/new/` | leaf fix | | `/organizations/:orgId/alerts/crons-rules/:projectId/:monitorSlug/` | leaf fix | ## Verification - [x] Each route above loads in the browser and renders inside a `
` element --------- Co-authored-by: Priscila Oliveira Co-authored-by: Claude Opus 4.6 --- static/app/views/alerts/create.tsx | 4 +- static/app/views/alerts/edit.tsx | 4 +- .../app/views/alerts/list/incidents/index.tsx | 66 ++--- .../alerts/list/rules/alertRulesList.tsx | 246 ++++++++--------- .../rules/issue/details/ruleDetails.tsx | 250 +++++++++--------- 5 files changed, 289 insertions(+), 281 deletions(-) diff --git a/static/app/views/alerts/create.tsx b/static/app/views/alerts/create.tsx index 8230782cf304cf..7179f1f4db1bcf 100644 --- a/static/app/views/alerts/create.tsx +++ b/static/app/views/alerts/create.tsx @@ -137,7 +137,7 @@ export default function Create() { const title = t('New Alert Rule'); return ( - + @@ -228,6 +228,6 @@ export default function Create() { )} - + ); } diff --git a/static/app/views/alerts/edit.tsx b/static/app/views/alerts/edit.tsx index 9805c6bbfb5c1c..a5dced1453d396 100644 --- a/static/app/views/alerts/edit.tsx +++ b/static/app/views/alerts/edit.tsx @@ -61,7 +61,7 @@ export default function ProjectAlertsEditor() { const {teams, isLoading: teamsLoading} = useUserTeams(); return ( - + )} - + ); } diff --git a/static/app/views/alerts/list/incidents/index.tsx b/static/app/views/alerts/list/incidents/index.tsx index b3d75f8a7cf9ca..eb2d9e0b008c59 100644 --- a/static/app/views/alerts/list/incidents/index.tsx +++ b/static/app/views/alerts/list/incidents/index.tsx @@ -263,28 +263,30 @@ class IncidentsList extends DeprecatedAsyncComponent< return ( - - - - - {!this.tryRenderOnboarding() && ( - - - {t('This page only shows metric alerts.')} - - - - )} - {this.renderList()} - - - + + + + + + {!this.tryRenderOnboarding() && ( + + + {t('This page only shows metric alerts.')} + + + + )} + {this.renderList()} + + + + ); } @@ -303,15 +305,17 @@ export default function IncidentsListContainer() { }, []); const renderDisabled = () => ( - - - - - {t("You don't have access to this feature")} - - - - + + + + + + {t("You don't have access to this feature")} + + + + + ); return ( diff --git a/static/app/views/alerts/list/rules/alertRulesList.tsx b/static/app/views/alerts/list/rules/alertRulesList.tsx index 1c5e300230d21b..8d37a4e8d81516 100644 --- a/static/app/views/alerts/list/rules/alertRulesList.tsx +++ b/static/app/views/alerts/list/rules/alertRulesList.tsx @@ -210,132 +210,134 @@ export default function AlertRulesList() { - - - - - - {!hasMetricAlertsFeature && hasAnyMetricAlerts && ( - - - Your metric alerts have been disabled. Upgrade your plan to re-enable - them. - - - )} - - - {t('Alert Rule')} {sort.field === 'name' ? sortArrow : null} - , - - {t('Status')} {isAlertRuleSort ? sortArrow : null} - , - t('Project'), - t('Team'), - t('Actions'), - ]} - > - {isError ? ( - - ) : null} - 0} + + + + + + + {!hasMetricAlertsFeature && hasAnyMetricAlerts && ( + + + Your metric alerts have been disabled. Upgrade your plan to re-enable + them. + + + )} + + + {t('Alert Rule')} {sort.field === 'name' ? sortArrow : null} + , + + {t('Status')} {isAlertRuleSort ? sortArrow : null} + , + t('Project'), + t('Team'), + t('Actions'), + ]} > - - {({initiallyLoaded, projects}) => - ruleList.map(rule => { - const isIssueAlertInstance = isIssueAlert(rule); - const keyPrefix = isIssueAlertInstance - ? AlertRuleType.ISSUE - : rule.type === CombinedAlertType.UPTIME - ? AlertRuleType.UPTIME - : AlertRuleType.METRIC; + {isError ? ( + + ) : null} + 0} + > + + {({initiallyLoaded, projects}) => + ruleList.map(rule => { + const isIssueAlertInstance = isIssueAlert(rule); + const keyPrefix = isIssueAlertInstance + ? AlertRuleType.ISSUE + : rule.type === CombinedAlertType.UPTIME + ? AlertRuleType.UPTIME + : AlertRuleType.METRIC; - return ( - - ); - }) + return ( + + ); + }) + } + + + + { + let team = currentQuery.team; + // Keep team parameter, but empty to remove parameters + if (!team || team.length === 0) { + team = ''; } - - - - { - let team = currentQuery.team; - // Keep team parameter, but empty to remove parameters - if (!team || team.length === 0) { - team = ''; - } - navigate({ - pathname: path, - query: {...currentQuery, team, cursor}, - }); - }} - /> - - - + navigate({ + pathname: path, + query: {...currentQuery, team, cursor}, + }); + }} + /> + + + + ); } diff --git a/static/app/views/alerts/rules/issue/details/ruleDetails.tsx b/static/app/views/alerts/rules/issue/details/ruleDetails.tsx index a32f1eb4f7922a..2040b0d26c7628 100644 --- a/static/app/views/alerts/rules/issue/details/ruleDetails.tsx +++ b/static/app/views/alerts/rules/issue/details/ruleDetails.tsx @@ -383,139 +383,141 @@ export default function AlertRuleDetails() { const {period, start, end, utc} = getDataDatetime(); const cursor = decodeScalar(location.query.cursor); return ( - - - - - - - - + + + + + + - {rule.name} - - - - - - {({hasAccess}) => ( - - )} - - } - to={duplicateLink} - disabled={rule.status === 'disabled'} - > - {t('Duplicate')} - - } - to={makeAlertsPathname({ - path: `/rules/${projectSlug}/${ruleId}/`, - organization, - })} - onClick={() => - trackAnalytics('issue_alert_rule_details.edit_clicked', { + + + {rule.name} + + + + + + {({hasAccess}) => ( + + )} + + } + to={duplicateLink} + disabled={rule.status === 'disabled'} + > + {t('Duplicate')} + + } + to={makeAlertsPathname({ + path: `/rules/${projectSlug}/${ruleId}/`, organization, - rule_id: parseInt(ruleId, 10), - }) - } - > - {rule.status === 'disabled' ? t('Edit to enable') : t('Edit Rule')} - - - - - - - - {renderIncompatibleAlert()} - {renderDisabledAlertBanner()} - {rule.snooze && ( - - {rule.snoozeForEveryone ? ( - - {tct( - "[creator] muted this alert for everyone so you won't get these notifications in the future.", - { - creator: rule.snoozeCreatedBy, - } - )} - - ) : ( - - )} - - )} - - - + trackAnalytics('issue_alert_rule_details.edit_clicked', { + organization, + rule_id: parseInt(ruleId, 10), + }) + } + > + {rule.status === 'disabled' ? t('Edit to enable') : t('Edit Rule')} + + + + + + + + {renderIncompatibleAlert()} + {renderDisabledAlertBanner()} + {rule.snooze && ( + + {rule.snoozeForEveryone ? ( + + {tct( + "[creator] muted this alert for everyone so you won't get these notifications in the future.", + { + creator: rule.snoozeCreatedBy, + } + )} + + ) : ( + + )} + + )} + + + + + - - - - - - - - + + + + + + + ); } From 9bcfd0fbf41989b28635819054aad5c6cb65fd0b Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 27 Mar 2026 04:03:11 -0700 Subject: [PATCH 02/13] ref(layout): use Layout.Page on isolated pages (#111644) Wraps non-compliant routes in isolated page areas with Layout.Page. Part of the Layout.Page audit remediation. ## Changes - `static/app/views/insights/pages/conversations/layout.tsx`: moved `Layout.Page` from `overview.tsx` to the layout component so it wraps both the header and outlet content - `static/app/views/insights/pages/conversations/overview.tsx`: removed `Layout.Page` (now handled by layout) - `static/app/views/profiling/continuousProfileProvider.tsx`: added `Layout.Page` wrapping header + outlet so flamegraph pages get correct framing - `static/app/views/profiling/transactionProfileProvider.tsx`: added `Layout.Page` wrapping header + outlet so flamechart pages get correct framing - `static/app/views/profiling/continuousProfileFlamegraph.tsx`: removed `Layout.Page` (now handled by provider) - `static/app/views/profiling/profileFlamechart.tsx`: removed `Layout.Page` (now handled by provider) - `static/app/views/profiling/profileSummary/index.tsx`: wrap `ProfileSummaryPageToggle` with `Layout.Page` - `static/app/views/profiling/differentialFlamegraph.tsx`: wrap `DifferentialFlamegraphWithProviders` with `Layout.Page` - `static/app/views/projectInstall/newProject.tsx`: wrap `NewProject` content with `Layout.Page` ## Affected routes | Path | Strategy | | --- | --- | | `/organizations/:orgId/explore/conversations/` | layout-level wrap (header + outlet) | | `/organizations/:orgId/profiling/profile/:projectId/flamegraph/` | provider-level wrap (header + outlet) | | `/organizations/:orgId/profiling/profile/:projectId/:eventId/flamegraph/` | provider-level wrap (header + outlet) | | `/organizations/:orgId/profiling/summary/:projectId/` | leaf fix | | `/organizations/:orgId/profiling/profile/:projectId/differential-flamegraph/` | leaf fix | | `/organizations/:orgId/projects/new/` | leaf fix | ## Verification - [ ] Each route above loads in the browser and renders inside a `
` element --------- Co-authored-by: Priscila Oliveira Co-authored-by: Claude Opus 4.6 --- .../insights/pages/conversations/layout.tsx | 6 +-- .../profiling/continuousProfileFlamegraph.tsx | 47 ++++++++----------- .../profiling/continuousProfileProvider.tsx | 15 +++--- .../profiling/differentialFlamegraph.tsx | 31 ++++++------ .../profiling/layoutPageWithHiddenFooter.tsx | 12 +++++ .../app/views/profiling/profileFlamechart.tsx | 47 ++++++++----------- .../views/profiling/profileSummary/index.tsx | 22 ++++----- .../profiling/transactionProfileProvider.tsx | 19 ++++---- .../app/views/projectInstall/newProject.tsx | 17 ++++--- 9 files changed, 106 insertions(+), 110 deletions(-) create mode 100644 static/app/views/profiling/layoutPageWithHiddenFooter.tsx diff --git a/static/app/views/insights/pages/conversations/layout.tsx b/static/app/views/insights/pages/conversations/layout.tsx index 30020aca1eae4a..9df28e8b02b4b9 100644 --- a/static/app/views/insights/pages/conversations/layout.tsx +++ b/static/app/views/insights/pages/conversations/layout.tsx @@ -1,6 +1,6 @@ -import {Fragment} from 'react'; import {Outlet, useMatches} from 'react-router-dom'; +import * as Layout from 'sentry/components/layouts/thirds'; import {ConversationsPageHeader} from 'sentry/views/insights/pages/conversations/conversationsPageHeader'; import {ModuleName} from 'sentry/views/insights/types'; @@ -8,12 +8,12 @@ function ConversationsLayout() { const handle = useMatches().at(-1)?.handle as {module?: ModuleName} | undefined; return ( - + {handle && 'module' in handle ? ( ) : null} - + ); } diff --git a/static/app/views/profiling/continuousProfileFlamegraph.tsx b/static/app/views/profiling/continuousProfileFlamegraph.tsx index 96f3fde4aae9a7..0aaa232b60861e 100644 --- a/static/app/views/profiling/continuousProfileFlamegraph.tsx +++ b/static/app/views/profiling/continuousProfileFlamegraph.tsx @@ -4,7 +4,6 @@ import * as qs from 'query-string'; import {Stack} from '@sentry/scraps/layout'; -import * as Layout from 'sentry/components/layouts/thirds'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {ContinuousFlamegraph} from 'sentry/components/profiling/flamegraph/continuousFlamegraph'; import {SentryDocumentTitle} from 'sentry/components/sentryDocumentTitle'; @@ -104,27 +103,25 @@ export default function ContinuousProfileFlamegraphWrapper() { title={t('Profiling \u2014 Flamechart')} orgSlug={organization.slug} > - - - - - - - - {profiles.type === 'loading' ? ( - - - - ) : null} - - - - - - + + + + + + + {profiles.type === 'loading' ? ( + + + + ) : null} + + + + + ); } @@ -159,9 +156,3 @@ const FlamegraphContainer = styled('div')` flex-direction: column; flex: 1 1 100%; `; - -const LayoutPageWithHiddenFooter = styled(Layout.Page)` - ~ footer { - display: none; - } -`; diff --git a/static/app/views/profiling/continuousProfileProvider.tsx b/static/app/views/profiling/continuousProfileProvider.tsx index d19eee233ec8a3..bbd1aec47844e8 100644 --- a/static/app/views/profiling/continuousProfileProvider.tsx +++ b/static/app/views/profiling/continuousProfileProvider.tsx @@ -9,6 +9,7 @@ import {decodeScalar} from 'sentry/utils/queryString'; import {useLocation} from 'sentry/utils/useLocation'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useParams} from 'sentry/utils/useParams'; +import {LayoutPageWithHiddenFooter} from 'sentry/views/profiling/layoutPageWithHiddenFooter'; import {ContinuousProfileProvider, ProfileTransactionContext} from './profilesProvider'; @@ -65,12 +66,14 @@ export default function ProfileAndTransactionProvider(): React.ReactElement { setProfile={setProfile} > - - + + + + ); diff --git a/static/app/views/profiling/differentialFlamegraph.tsx b/static/app/views/profiling/differentialFlamegraph.tsx index d0d5a91d143383..3d704d0e81bd63 100644 --- a/static/app/views/profiling/differentialFlamegraph.tsx +++ b/static/app/views/profiling/differentialFlamegraph.tsx @@ -40,6 +40,7 @@ import {FlamegraphRenderer2D} from 'sentry/utils/profiling/renderers/flamegraphR import {FlamegraphRendererWebGL} from 'sentry/utils/profiling/renderers/flamegraphRendererWebGL'; import {Rect} from 'sentry/utils/profiling/speedscope'; import {useLocation} from 'sentry/utils/useLocation'; +import {LayoutPageWithHiddenFooter} from 'sentry/views/profiling/layoutPageWithHiddenFooter'; import {LOADING_PROFILE_GROUP} from 'sentry/views/profiling/profileGroupProvider'; const PROFILE_TYPE = 'differential aggregate flamegraph' as const; @@ -348,26 +349,24 @@ const DifferentialFlamegraphContainer = styled('div')` display: flex; flex-direction: column; flex: 1; - - ~ footer { - display: none; - } `; function DifferentialFlamegraphWithProviders() { return ( - - - - - + + + + + + + ); } diff --git a/static/app/views/profiling/layoutPageWithHiddenFooter.tsx b/static/app/views/profiling/layoutPageWithHiddenFooter.tsx new file mode 100644 index 00000000000000..ff8ace1842eae0 --- /dev/null +++ b/static/app/views/profiling/layoutPageWithHiddenFooter.tsx @@ -0,0 +1,12 @@ +import styled from '@emotion/styled'; + +import * as Layout from 'sentry/components/layouts/thirds'; + +// The footer component is a sibling of this div. +// Remove it so the flamegraph can take up the +// entire screen. +export const LayoutPageWithHiddenFooter = styled(Layout.Page)` + ~ footer { + display: none; + } +`; diff --git a/static/app/views/profiling/profileFlamechart.tsx b/static/app/views/profiling/profileFlamechart.tsx index 4ed950e7e986fb..6c43383b1d6380 100644 --- a/static/app/views/profiling/profileFlamechart.tsx +++ b/static/app/views/profiling/profileFlamechart.tsx @@ -4,7 +4,6 @@ import * as qs from 'query-string'; import {Stack} from '@sentry/scraps/layout'; -import * as Layout from 'sentry/components/layouts/thirds'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {Flamegraph} from 'sentry/components/profiling/flamegraph/flamegraph'; import {SentryDocumentTitle} from 'sentry/components/sentryDocumentTitle'; @@ -105,27 +104,25 @@ export default function ProfileFlamegraphWrapper() { title={t('Profiling \u2014 Flamechart')} orgSlug={organization.slug} > - - - - - - - - {profiles.type === 'loading' || profiledTransaction.type === 'loading' ? ( - - - - ) : null} - - - - - - + + + + + + + {profiles.type === 'loading' || profiledTransaction.type === 'loading' ? ( + + + + ) : null} + + + + + ); } @@ -160,9 +157,3 @@ const FlamegraphContainer = styled('div')` flex-direction: column; flex: 1 1 100%; `; - -const LayoutPageWithHiddenFooter = styled(Layout.Page)` - ~ footer { - display: none; - } -`; diff --git a/static/app/views/profiling/profileSummary/index.tsx b/static/app/views/profiling/profileSummary/index.tsx index 1e56955c2e3f31..d4137d3717c93d 100644 --- a/static/app/views/profiling/profileSummary/index.tsx +++ b/static/app/views/profiling/profileSummary/index.tsx @@ -67,6 +67,7 @@ import { useFlamegraph, } from 'sentry/views/profiling/flamegraphProvider'; import {ProfilesSummaryChart} from 'sentry/views/profiling/landing/profilesSummaryChart'; +import {LayoutPageWithHiddenFooter} from 'sentry/views/profiling/layoutPageWithHiddenFooter'; import {ProfileGroupProvider} from 'sentry/views/profiling/profileGroupProvider'; import {ProfilesTable} from 'sentry/views/profiling/profileSummary/profilesTable'; @@ -636,15 +637,6 @@ const ProfileSummaryContainer = styled('div')` display: flex; flex-direction: column; flex: 1 1 100%; - - /* - * The footer component is a sibling of this div. - * Remove it so the flamegraph can take up the - * entire screen. - */ - ~ footer { - display: none; - } `; const PROFILE_DIGEST_FIELDS = [ @@ -781,10 +773,12 @@ const ProfileDigestLabel = styled('span')` export default function ProfileSummaryPageToggle() { return ( - - - - - + + + + + + + ); } diff --git a/static/app/views/profiling/transactionProfileProvider.tsx b/static/app/views/profiling/transactionProfileProvider.tsx index f7fb3553b06802..34a810413dc3ff 100644 --- a/static/app/views/profiling/transactionProfileProvider.tsx +++ b/static/app/views/profiling/transactionProfileProvider.tsx @@ -8,6 +8,7 @@ import {isSchema, isSentrySampledProfile} from 'sentry/utils/profiling/guards/pr import {useSentryEvent} from 'sentry/utils/profiling/hooks/useSentryEvent'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useParams} from 'sentry/utils/useParams'; +import {LayoutPageWithHiddenFooter} from 'sentry/views/profiling/layoutPageWithHiddenFooter'; import {ProfileTransactionContext, TransactionProfileProvider} from './profilesProvider'; @@ -46,14 +47,16 @@ export default function ProfileAndTransactionProvider(): React.ReactElement { setProfile={setProfile} > - - + + + + ); diff --git a/static/app/views/projectInstall/newProject.tsx b/static/app/views/projectInstall/newProject.tsx index 57ccb755247948..19c5ea0d47685a 100644 --- a/static/app/views/projectInstall/newProject.tsx +++ b/static/app/views/projectInstall/newProject.tsx @@ -1,5 +1,6 @@ import styled from '@emotion/styled'; +import * as Layout from 'sentry/components/layouts/thirds'; import {SentryDocumentTitle} from 'sentry/components/sentryDocumentTitle'; import {CreateProject} from './createProject'; @@ -7,13 +8,15 @@ import {CreateProject} from './createProject'; function NewProject() { return ( - -
- - - -
- + + +
+ + + +
+ + ); } From 939e726b947fa0a82c53617ccfd0581ddbbf07ab Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Fri, 27 Mar 2026 12:37:18 +0000 Subject: [PATCH 03/13] Revert "ref(ui): Remove overflow from guide steps (#111462)" This reverts commit 347562e411d73be37e0b7e697946f2ee3fe87573. Co-authored-by: priscilawebdev <29228205+priscilawebdev@users.noreply.github.com> --- static/app/components/guidedSteps/guidedSteps.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/components/guidedSteps/guidedSteps.tsx b/static/app/components/guidedSteps/guidedSteps.tsx index 730f217d20d0a8..113e2dc8497db6 100644 --- a/static/app/components/guidedSteps/guidedSteps.tsx +++ b/static/app/components/guidedSteps/guidedSteps.tsx @@ -371,6 +371,7 @@ const ChildrenWrapper = styled('div')<{isActive: boolean}>` `; const StepDetails = styled('div')` + overflow: hidden; grid-area: details; `; From b0faaaaacd1b0f81a47ed44a5b6e55771d281748 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 27 Mar 2026 08:32:23 -0500 Subject: [PATCH 04/13] ref(scm): Move protocol definitions to types.py (#111705) Protocol definitions are expected to be imported by the end-user. They should be in a public module. `types.py` makes the most sense. --- src/sentry/scm/actions.py | 42 +- src/sentry/scm/private/facade.py | 3 +- src/sentry/scm/private/helpers.py | 3 +- src/sentry/scm/private/provider.py | 611 ---------------------- src/sentry/scm/types.py | 576 +++++++++++++++++++- tests/sentry/scm/test_fixtures.py | 2 +- tests/sentry/scm/unit/test_scm_actions.py | 10 +- 7 files changed, 606 insertions(+), 641 deletions(-) delete mode 100644 src/sentry/scm/private/provider.py diff --git a/src/sentry/scm/actions.py b/src/sentry/scm/actions.py index b7689cd6bd25cc..c4d6c0041a79bc 100644 --- a/src/sentry/scm/actions.py +++ b/src/sentry/scm/actions.py @@ -13,8 +13,20 @@ map_repository_model_to_repository, ) from sentry.scm.private.ipc import record_count_metric -from sentry.scm.private.provider import ( +from sentry.scm.private.rate_limit import RateLimitProvider +from sentry.scm.types import ( ALL_PROTOCOLS, + SHA, + ActionResult, + ArchiveFormat, + ArchiveLink, + BranchName, + BuildConclusion, + BuildStatus, + CheckRun, + CheckRunOutput, + Comment, + Commit, CompareCommitsProtocol, CreateBranchProtocol, CreateCheckRunProtocol, @@ -38,6 +50,7 @@ DeletePullRequestCommentProtocol, DeletePullRequestCommentReactionProtocol, DeletePullRequestReactionProtocol, + FileContent, GetArchiveLinkProtocol, GetBranchProtocol, GetCheckRunProtocol, @@ -58,34 +71,15 @@ GetPullRequestReactionsProtocol, GetPullRequestsProtocol, GetTreeProtocol, - MinimizeCommentProtocol, - Provider, - RequestReviewProtocol, - UpdateBranchProtocol, - UpdateCheckRunProtocol, - UpdatePullRequestProtocol, -) -from sentry.scm.private.rate_limit import RateLimitProvider -from sentry.scm.types import ( - SHA, - ActionResult, - ArchiveFormat, - ArchiveLink, - BranchName, - BuildConclusion, - BuildStatus, - CheckRun, - CheckRunOutput, - Comment, - Commit, - FileContent, GitBlob, GitCommitObject, GitRef, GitTree, InputTreeEntry, + MinimizeCommentProtocol, PaginatedActionResult, PaginationParams, + Provider, PullRequest, PullRequestCommit, PullRequestFile, @@ -96,12 +90,16 @@ Repository, RepositoryId, RequestOptions, + RequestReviewProtocol, ResourceId, Review, ReviewComment, ReviewCommentInput, ReviewEvent, ReviewSide, + UpdateBranchProtocol, + UpdateCheckRunProtocol, + UpdatePullRequestProtocol, ) diff --git a/src/sentry/scm/private/facade.py b/src/sentry/scm/private/facade.py index e915adee68fa33..5d15cc7904ac9a 100644 --- a/src/sentry/scm/private/facade.py +++ b/src/sentry/scm/private/facade.py @@ -6,8 +6,7 @@ from sentry.scm.private.helpers import exec_provider_fn from sentry.scm.private.ipc import record_count_metric -from sentry.scm.private.provider import ALL_PROTOCOLS, Provider -from sentry.scm.types import Referrer +from sentry.scm.types import ALL_PROTOCOLS, Provider, Referrer def _delegating_method(name: str) -> Callable[..., Any]: diff --git a/src/sentry/scm/private/helpers.py b/src/sentry/scm/private/helpers.py index 4f0ae6f2f6509e..1ca94c1a9f9e07 100644 --- a/src/sentry/scm/private/helpers.py +++ b/src/sentry/scm/private/helpers.py @@ -8,11 +8,10 @@ from sentry.models.repository import Repository as RepositoryModel from sentry.scm.errors import SCMCodedError, SCMError, SCMUnhandledException from sentry.scm.private.ipc import record_count_metric -from sentry.scm.private.provider import Provider from sentry.scm.private.providers.github import GitHubProvider from sentry.scm.private.providers.gitlab import GitLabProvider from sentry.scm.private.rate_limit import RateLimitProvider, RedisRateLimitProvider -from sentry.scm.types import ExternalId, ProviderName, Referrer, Repository, RepositoryId +from sentry.scm.types import ExternalId, Provider, ProviderName, Referrer, Repository, RepositoryId def map_integration_to_provider( diff --git a/src/sentry/scm/private/provider.py b/src/sentry/scm/private/provider.py deleted file mode 100644 index 502494f5068b10..00000000000000 --- a/src/sentry/scm/private/provider.py +++ /dev/null @@ -1,611 +0,0 @@ -from typing import Protocol, runtime_checkable - -from sentry.scm.types import ( - SHA, - ActionResult, - ArchiveFormat, - ArchiveLink, - BranchName, - BuildConclusion, - BuildStatus, - CheckRun, - CheckRunOutput, - Comment, - Commit, - FileContent, - GitBlob, - GitCommitObject, - GitRef, - GitTree, - InputTreeEntry, - PaginatedActionResult, - PaginationParams, - PullRequest, - PullRequestCommit, - PullRequestFile, - PullRequestState, - Reaction, - ReactionResult, - Referrer, - Repository, - RequestOptions, - ResourceId, - Review, - ReviewComment, - ReviewCommentInput, - ReviewEvent, - ReviewSide, -) - -# Issue Comment Protocols - - -@runtime_checkable -class GetIssueCommentsProtocol(Protocol): - def get_issue_comments( - self, - issue_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[Comment]: ... - - -@runtime_checkable -class CreateIssueCommentProtocol(Protocol): - def create_issue_comment(self, issue_id: str, body: str) -> ActionResult[Comment]: ... - - -@runtime_checkable -class DeleteIssueCommentProtocol(Protocol): - def delete_issue_comment(self, issue_id: str, comment_id: str) -> None: ... - - -# Pull Request Comment Protocols - - -@runtime_checkable -class GetPullRequestCommentsProtocol(Protocol): - def get_pull_request_comments( - self, - pull_request_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[Comment]: ... - - -@runtime_checkable -class CreatePullRequestCommentProtocol(Protocol): - def create_pull_request_comment( - self, pull_request_id: str, body: str - ) -> ActionResult[Comment]: ... - - -@runtime_checkable -class DeletePullRequestCommentProtocol(Protocol): - def delete_pull_request_comment(self, pull_request_id: str, comment_id: str) -> None: ... - - -# Issue Comment Reaction Protocols - - -@runtime_checkable -class GetIssueCommentReactionsProtocol(Protocol): - def get_issue_comment_reactions( - self, - issue_id: str, - comment_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[ReactionResult]: ... - - -@runtime_checkable -class CreateIssueCommentReactionProtocol(Protocol): - def create_issue_comment_reaction( - self, issue_id: str, comment_id: str, reaction: Reaction - ) -> ActionResult[ReactionResult]: ... - - -@runtime_checkable -class DeleteIssueCommentReactionProtocol(Protocol): - def delete_issue_comment_reaction( - self, issue_id: str, comment_id: str, reaction_id: str - ) -> None: ... - - -# Pull Request Comment Reaction Protocols - - -@runtime_checkable -class GetPullRequestCommentReactionsProtocol(Protocol): - def get_pull_request_comment_reactions( - self, - pull_request_id: str, - comment_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[ReactionResult]: ... - - -@runtime_checkable -class CreatePullRequestCommentReactionProtocol(Protocol): - def create_pull_request_comment_reaction( - self, pull_request_id: str, comment_id: str, reaction: Reaction - ) -> ActionResult[ReactionResult]: ... - - -@runtime_checkable -class DeletePullRequestCommentReactionProtocol(Protocol): - def delete_pull_request_comment_reaction( - self, pull_request_id: str, comment_id: str, reaction_id: str - ) -> None: ... - - -# Issue Reaction Protocols - - -@runtime_checkable -class GetIssueReactionsProtocol(Protocol): - def get_issue_reactions( - self, - issue_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[ReactionResult]: ... - - -@runtime_checkable -class CreateIssueReactionProtocol(Protocol): - def create_issue_reaction( - self, issue_id: str, reaction: Reaction - ) -> ActionResult[ReactionResult]: ... - - -@runtime_checkable -class DeleteIssueReactionProtocol(Protocol): - def delete_issue_reaction(self, issue_id: str, reaction_id: str) -> None: ... - - -# Pull Request Reaction Protocols - - -@runtime_checkable -class GetPullRequestReactionsProtocol(Protocol): - def get_pull_request_reactions( - self, - pull_request_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[ReactionResult]: ... - - -@runtime_checkable -class CreatePullRequestReactionProtocol(Protocol): - def create_pull_request_reaction( - self, pull_request_id: str, reaction: Reaction - ) -> ActionResult[ReactionResult]: ... - - -@runtime_checkable -class DeletePullRequestReactionProtocol(Protocol): - def delete_pull_request_reaction(self, pull_request_id: str, reaction_id: str) -> None: ... - - -# Branch Protocols - - -@runtime_checkable -class GetBranchProtocol(Protocol): - def get_branch( - self, - branch: BranchName, - request_options: RequestOptions | None = None, - ) -> ActionResult[GitRef]: ... - - -@runtime_checkable -class CreateBranchProtocol(Protocol): - def create_branch(self, branch: BranchName, sha: SHA) -> ActionResult[GitRef]: ... - - -@runtime_checkable -class UpdateBranchProtocol(Protocol): - def update_branch( - self, branch: BranchName, sha: SHA, force: bool = False - ) -> ActionResult[GitRef]: ... - - -# Commit Protocols - - -@runtime_checkable -class GetCommitProtocol(Protocol): - def get_commit( - self, - sha: SHA, - request_options: RequestOptions | None = None, - ) -> ActionResult[Commit]: ... - - -@runtime_checkable -class GetCommitsProtocol(Protocol): - def get_commits( - self, - ref: str | None = None, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[Commit]: ... - - -@runtime_checkable -class GetCommitsByPathProtocol(Protocol): - def get_commits_by_path( - self, - path: str, - ref: str | None = None, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[Commit]: ... - - -@runtime_checkable -class CompareCommitsProtocol(Protocol): - def compare_commits( - self, - start_sha: SHA, - end_sha: SHA, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[Commit]: ... - - -# Pull Request Protocols - - -@runtime_checkable -class GetPullRequestProtocol(Protocol): - def get_pull_request( - self, - pull_request_id: str, - request_options: RequestOptions | None = None, - ) -> ActionResult[PullRequest]: ... - - -@runtime_checkable -class GetPullRequestsProtocol(Protocol): - def get_pull_requests( - self, - state: PullRequestState | None = "open", - head: BranchName | None = None, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[PullRequest]: ... - - -@runtime_checkable -class GetPullRequestFilesProtocol(Protocol): - def get_pull_request_files( - self, - pull_request_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[PullRequestFile]: ... - - -@runtime_checkable -class GetPullRequestCommitsProtocol(Protocol): - def get_pull_request_commits( - self, - pull_request_id: str, - pagination: PaginationParams | None = None, - request_options: RequestOptions | None = None, - ) -> PaginatedActionResult[PullRequestCommit]: ... - - -@runtime_checkable -class GetPullRequestDiffProtocol(Protocol): - def get_pull_request_diff( - self, - pull_request_id: str, - request_options: RequestOptions | None = None, - ) -> ActionResult[str]: ... - - -@runtime_checkable -class CreatePullRequestProtocol(Protocol): - def create_pull_request( - self, - title: str, - body: str, - head: BranchName, - base: BranchName, - ) -> ActionResult[PullRequest]: ... - - -@runtime_checkable -class CreatePullRequestDraftProtocol(Protocol): - def create_pull_request_draft( - self, - title: str, - body: str, - head: BranchName, - base: BranchName, - ) -> ActionResult[PullRequest]: ... - - -@runtime_checkable -class UpdatePullRequestProtocol(Protocol): - def update_pull_request( - self, - pull_request_id: str, - title: str | None = None, - body: str | None = None, - state: PullRequestState | None = None, - ) -> ActionResult[PullRequest]: ... - - -@runtime_checkable -class RequestReviewProtocol(Protocol): - def request_review(self, pull_request_id: str, reviewers: list[str]) -> None: ... - - -# Git Object Protocols - - -@runtime_checkable -class GetTreeProtocol(Protocol): - def get_tree( - self, - tree_sha: SHA, - recursive: bool = True, - request_options: RequestOptions | None = None, - ) -> ActionResult[GitTree]: ... - - -@runtime_checkable -class GetGitCommitProtocol(Protocol): - def get_git_commit( - self, - sha: SHA, - request_options: RequestOptions | None = None, - ) -> ActionResult[GitCommitObject]: ... - - -@runtime_checkable -class CreateGitBlobProtocol(Protocol): - def create_git_blob(self, content: str, encoding: str) -> ActionResult[GitBlob]: ... - - -@runtime_checkable -class CreateGitTreeProtocol(Protocol): - def create_git_tree( - self, tree: list[InputTreeEntry], base_tree: SHA | None = None - ) -> ActionResult[GitTree]: ... - - -@runtime_checkable -class CreateGitCommitProtocol(Protocol): - def create_git_commit( - self, message: str, tree_sha: SHA, parent_shas: list[SHA] - ) -> ActionResult[GitCommitObject]: ... - - -# File Content Protocol - - -@runtime_checkable -class GetFileContentProtocol(Protocol): - def get_file_content( - self, - path: str, - ref: str | None = None, - request_options: RequestOptions | None = None, - ) -> ActionResult[FileContent]: ... - - -# Archive Protocols - - -@runtime_checkable -class GetArchiveLinkProtocol(Protocol): - def get_archive_link( - self, - ref: str, - archive_format: ArchiveFormat = "tarball", - ) -> ActionResult[ArchiveLink]: ... - - -# Check Run Protocols - - -@runtime_checkable -class GetCheckRunProtocol(Protocol): - def get_check_run( - self, - check_run_id: ResourceId, - request_options: RequestOptions | None = None, - ) -> ActionResult[CheckRun]: ... - - -@runtime_checkable -class CreateCheckRunProtocol(Protocol): - def create_check_run( - self, - name: str, - head_sha: SHA, - status: BuildStatus | None = None, - conclusion: BuildConclusion | None = None, - external_id: str | None = None, - started_at: str | None = None, - completed_at: str | None = None, - output: CheckRunOutput | None = None, - ) -> ActionResult[CheckRun]: ... - - -@runtime_checkable -class UpdateCheckRunProtocol(Protocol): - def update_check_run( - self, - check_run_id: ResourceId, - status: BuildStatus | None = None, - conclusion: BuildConclusion | None = None, - output: CheckRunOutput | None = None, - ) -> ActionResult[CheckRun]: ... - - -# Review Protocols - - -@runtime_checkable -class CreateReviewCommentFileProtocol(Protocol): - def create_review_comment_file( - self, - pull_request_id: str, - commit_id: SHA, - body: str, - path: str, - side: ReviewSide, - ) -> ActionResult[ReviewComment]: ... - - -@runtime_checkable -class CreateReviewCommentLineProtocol(Protocol): - def create_review_comment_line( - self, - pull_request_id: str, - commit_id: SHA, - body: str, - path: str, - line: int, - side: ReviewSide, - ) -> ActionResult[ReviewComment]: ... - - -@runtime_checkable -class CreateReviewCommentMultilineProtocol(Protocol): - def create_review_comment_multiline( - self, - pull_request_id: str, - commit_id: SHA, - body: str, - path: str, - start_line: int, - start_side: ReviewSide, - end_line: int, - end_side: ReviewSide, - ) -> ActionResult[ReviewComment]: ... - - -@runtime_checkable -class CreateReviewCommentReplyProtocol(Protocol): - def create_review_comment_reply( - self, - pull_request_id: str, - body: str, - comment_id: str, - ) -> ActionResult[ReviewComment]: ... - - -@runtime_checkable -class CreateReviewProtocol(Protocol): - def create_review( - self, - pull_request_id: str, - commit_sha: SHA, - event: ReviewEvent, - comments: list[ReviewCommentInput], - body: str | None = None, - ) -> ActionResult[Review]: ... - - -# Moderation Protocols - - -@runtime_checkable -class MinimizeCommentProtocol(Protocol): - def minimize_comment(self, comment_node_id: str, reason: str) -> None: ... - - -@runtime_checkable -class ResolveReviewThreadProtocol(Protocol): - def resolve_review_thread(self, thread_node_id: str) -> None: ... - - -ALL_PROTOCOLS = ( - CompareCommitsProtocol, - CreateBranchProtocol, - CreateCheckRunProtocol, - CreateGitBlobProtocol, - CreateGitCommitProtocol, - CreateGitTreeProtocol, - CreateIssueCommentProtocol, - CreateIssueCommentReactionProtocol, - CreateIssueReactionProtocol, - CreatePullRequestCommentProtocol, - CreatePullRequestCommentReactionProtocol, - CreatePullRequestDraftProtocol, - CreatePullRequestProtocol, - CreatePullRequestReactionProtocol, - CreateReviewCommentFileProtocol, - CreateReviewCommentLineProtocol, - CreateReviewCommentMultilineProtocol, - CreateReviewCommentReplyProtocol, - CreateReviewProtocol, - DeleteIssueCommentProtocol, - DeleteIssueCommentReactionProtocol, - DeleteIssueReactionProtocol, - DeletePullRequestCommentProtocol, - DeletePullRequestCommentReactionProtocol, - DeletePullRequestReactionProtocol, - GetArchiveLinkProtocol, - GetBranchProtocol, - GetCheckRunProtocol, - GetCommitProtocol, - GetCommitsByPathProtocol, - GetCommitsProtocol, - GetFileContentProtocol, - GetGitCommitProtocol, - GetIssueCommentReactionsProtocol, - GetIssueCommentsProtocol, - GetIssueReactionsProtocol, - GetPullRequestCommentReactionsProtocol, - GetPullRequestCommentsProtocol, - GetPullRequestCommitsProtocol, - GetPullRequestDiffProtocol, - GetPullRequestFilesProtocol, - GetPullRequestProtocol, - GetPullRequestReactionsProtocol, - GetPullRequestsProtocol, - GetTreeProtocol, - MinimizeCommentProtocol, - RequestReviewProtocol, - ResolveReviewThreadProtocol, - UpdateBranchProtocol, - UpdateCheckRunProtocol, - UpdatePullRequestProtocol, -) - - -class Provider(Protocol): - """ - Providers abstract over an integration. They map generic commands to service-provider specific - commands and they map the results of those commands to generic result-types. - - Providers necessarily offer a larger API surface than what is available in an integration. Some - methods may be duplicates in some providers. This is intentional. Providers capture programmer - intent and translate it into a concrete interface. Therefore, providers provide a large range - of behaviors which may or may not be explicitly defined on a service-provider. - - Providers, also by necessity, offer a smaller API surface than what the SCM platform can - maximally provide. There are simply some operations which can not be adequately translated - between providers. None the less, we want to have a service-agnostic interface. This problem - is solved with capability-object-like system. Capabilities are progressively opted into using - structural sub-typing. As a provider's surface area expands the SourceCodeManager class will - automatically recognize that the provider has a particular capability and return "true" when - handling "can" requests. - """ - - organization_id: int - repository: Repository - - def is_rate_limited(self, referrer: Referrer) -> bool: ... diff --git a/src/sentry/scm/types.py b/src/sentry/scm/types.py index 7a5f6119db3ae9..d16c3b899b52b9 100644 --- a/src/sentry/scm/types.py +++ b/src/sentry/scm/types.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from datetime import datetime -from typing import Any, Literal, Required, TypedDict +from typing import Any, Literal, Protocol, Required, TypedDict, runtime_checkable type Action = Literal["check_run", "comment", "pull_request"] type EventType = "CheckRunEvent" | "CommentEvent" | "PullRequestEvent" @@ -584,3 +584,577 @@ class PullRequestEvent: the listener. In some cases, Sentry will query the database for information. This information is stored in the "sentry_meta" field and is accessible without performing redundant queries. """ + + +# Issue Comment Protocols + + +@runtime_checkable +class GetIssueCommentsProtocol(Protocol): + def get_issue_comments( + self, + issue_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[Comment]: ... + + +@runtime_checkable +class CreateIssueCommentProtocol(Protocol): + def create_issue_comment(self, issue_id: str, body: str) -> ActionResult[Comment]: ... + + +@runtime_checkable +class DeleteIssueCommentProtocol(Protocol): + def delete_issue_comment(self, issue_id: str, comment_id: str) -> None: ... + + +# Pull Request Comment Protocols + + +@runtime_checkable +class GetPullRequestCommentsProtocol(Protocol): + def get_pull_request_comments( + self, + pull_request_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[Comment]: ... + + +@runtime_checkable +class CreatePullRequestCommentProtocol(Protocol): + def create_pull_request_comment( + self, pull_request_id: str, body: str + ) -> ActionResult[Comment]: ... + + +@runtime_checkable +class DeletePullRequestCommentProtocol(Protocol): + def delete_pull_request_comment(self, pull_request_id: str, comment_id: str) -> None: ... + + +# Issue Comment Reaction Protocols + + +@runtime_checkable +class GetIssueCommentReactionsProtocol(Protocol): + def get_issue_comment_reactions( + self, + issue_id: str, + comment_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[ReactionResult]: ... + + +@runtime_checkable +class CreateIssueCommentReactionProtocol(Protocol): + def create_issue_comment_reaction( + self, issue_id: str, comment_id: str, reaction: Reaction + ) -> ActionResult[ReactionResult]: ... + + +@runtime_checkable +class DeleteIssueCommentReactionProtocol(Protocol): + def delete_issue_comment_reaction( + self, issue_id: str, comment_id: str, reaction_id: str + ) -> None: ... + + +# Pull Request Comment Reaction Protocols + + +@runtime_checkable +class GetPullRequestCommentReactionsProtocol(Protocol): + def get_pull_request_comment_reactions( + self, + pull_request_id: str, + comment_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[ReactionResult]: ... + + +@runtime_checkable +class CreatePullRequestCommentReactionProtocol(Protocol): + def create_pull_request_comment_reaction( + self, pull_request_id: str, comment_id: str, reaction: Reaction + ) -> ActionResult[ReactionResult]: ... + + +@runtime_checkable +class DeletePullRequestCommentReactionProtocol(Protocol): + def delete_pull_request_comment_reaction( + self, pull_request_id: str, comment_id: str, reaction_id: str + ) -> None: ... + + +# Issue Reaction Protocols + + +@runtime_checkable +class GetIssueReactionsProtocol(Protocol): + def get_issue_reactions( + self, + issue_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[ReactionResult]: ... + + +@runtime_checkable +class CreateIssueReactionProtocol(Protocol): + def create_issue_reaction( + self, issue_id: str, reaction: Reaction + ) -> ActionResult[ReactionResult]: ... + + +@runtime_checkable +class DeleteIssueReactionProtocol(Protocol): + def delete_issue_reaction(self, issue_id: str, reaction_id: str) -> None: ... + + +# Pull Request Reaction Protocols + + +@runtime_checkable +class GetPullRequestReactionsProtocol(Protocol): + def get_pull_request_reactions( + self, + pull_request_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[ReactionResult]: ... + + +@runtime_checkable +class CreatePullRequestReactionProtocol(Protocol): + def create_pull_request_reaction( + self, pull_request_id: str, reaction: Reaction + ) -> ActionResult[ReactionResult]: ... + + +@runtime_checkable +class DeletePullRequestReactionProtocol(Protocol): + def delete_pull_request_reaction(self, pull_request_id: str, reaction_id: str) -> None: ... + + +# Branch Protocols + + +@runtime_checkable +class GetBranchProtocol(Protocol): + def get_branch( + self, + branch: BranchName, + request_options: RequestOptions | None = None, + ) -> ActionResult[GitRef]: ... + + +@runtime_checkable +class CreateBranchProtocol(Protocol): + def create_branch(self, branch: BranchName, sha: SHA) -> ActionResult[GitRef]: ... + + +@runtime_checkable +class UpdateBranchProtocol(Protocol): + def update_branch( + self, branch: BranchName, sha: SHA, force: bool = False + ) -> ActionResult[GitRef]: ... + + +# Commit Protocols + + +@runtime_checkable +class GetCommitProtocol(Protocol): + def get_commit( + self, + sha: SHA, + request_options: RequestOptions | None = None, + ) -> ActionResult[Commit]: ... + + +@runtime_checkable +class GetCommitsProtocol(Protocol): + def get_commits( + self, + ref: str | None = None, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[Commit]: ... + + +@runtime_checkable +class GetCommitsByPathProtocol(Protocol): + def get_commits_by_path( + self, + path: str, + ref: str | None = None, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[Commit]: ... + + +@runtime_checkable +class CompareCommitsProtocol(Protocol): + def compare_commits( + self, + start_sha: SHA, + end_sha: SHA, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[Commit]: ... + + +# Pull Request Protocols + + +@runtime_checkable +class GetPullRequestProtocol(Protocol): + def get_pull_request( + self, + pull_request_id: str, + request_options: RequestOptions | None = None, + ) -> ActionResult[PullRequest]: ... + + +@runtime_checkable +class GetPullRequestsProtocol(Protocol): + def get_pull_requests( + self, + state: PullRequestState | None = "open", + head: BranchName | None = None, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[PullRequest]: ... + + +@runtime_checkable +class GetPullRequestFilesProtocol(Protocol): + def get_pull_request_files( + self, + pull_request_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[PullRequestFile]: ... + + +@runtime_checkable +class GetPullRequestCommitsProtocol(Protocol): + def get_pull_request_commits( + self, + pull_request_id: str, + pagination: PaginationParams | None = None, + request_options: RequestOptions | None = None, + ) -> PaginatedActionResult[PullRequestCommit]: ... + + +@runtime_checkable +class GetPullRequestDiffProtocol(Protocol): + def get_pull_request_diff( + self, + pull_request_id: str, + request_options: RequestOptions | None = None, + ) -> ActionResult[str]: ... + + +@runtime_checkable +class CreatePullRequestProtocol(Protocol): + def create_pull_request( + self, + title: str, + body: str, + head: BranchName, + base: BranchName, + ) -> ActionResult[PullRequest]: ... + + +@runtime_checkable +class CreatePullRequestDraftProtocol(Protocol): + def create_pull_request_draft( + self, + title: str, + body: str, + head: BranchName, + base: BranchName, + ) -> ActionResult[PullRequest]: ... + + +@runtime_checkable +class UpdatePullRequestProtocol(Protocol): + def update_pull_request( + self, + pull_request_id: str, + title: str | None = None, + body: str | None = None, + state: PullRequestState | None = None, + ) -> ActionResult[PullRequest]: ... + + +@runtime_checkable +class RequestReviewProtocol(Protocol): + def request_review(self, pull_request_id: str, reviewers: list[str]) -> None: ... + + +# Git Object Protocols + + +@runtime_checkable +class GetTreeProtocol(Protocol): + def get_tree( + self, + tree_sha: SHA, + recursive: bool = True, + request_options: RequestOptions | None = None, + ) -> ActionResult[GitTree]: ... + + +@runtime_checkable +class GetGitCommitProtocol(Protocol): + def get_git_commit( + self, + sha: SHA, + request_options: RequestOptions | None = None, + ) -> ActionResult[GitCommitObject]: ... + + +@runtime_checkable +class CreateGitBlobProtocol(Protocol): + def create_git_blob(self, content: str, encoding: str) -> ActionResult[GitBlob]: ... + + +@runtime_checkable +class CreateGitTreeProtocol(Protocol): + def create_git_tree( + self, tree: list[InputTreeEntry], base_tree: SHA | None = None + ) -> ActionResult[GitTree]: ... + + +@runtime_checkable +class CreateGitCommitProtocol(Protocol): + def create_git_commit( + self, message: str, tree_sha: SHA, parent_shas: list[SHA] + ) -> ActionResult[GitCommitObject]: ... + + +# File Content Protocol + + +@runtime_checkable +class GetFileContentProtocol(Protocol): + def get_file_content( + self, + path: str, + ref: str | None = None, + request_options: RequestOptions | None = None, + ) -> ActionResult[FileContent]: ... + + +# Archive Protocols + + +@runtime_checkable +class GetArchiveLinkProtocol(Protocol): + def get_archive_link( + self, + ref: str, + archive_format: ArchiveFormat = "tarball", + ) -> ActionResult[ArchiveLink]: ... + + +# Check Run Protocols + + +@runtime_checkable +class GetCheckRunProtocol(Protocol): + def get_check_run( + self, + check_run_id: ResourceId, + request_options: RequestOptions | None = None, + ) -> ActionResult[CheckRun]: ... + + +@runtime_checkable +class CreateCheckRunProtocol(Protocol): + def create_check_run( + self, + name: str, + head_sha: SHA, + status: BuildStatus | None = None, + conclusion: BuildConclusion | None = None, + external_id: str | None = None, + started_at: str | None = None, + completed_at: str | None = None, + output: CheckRunOutput | None = None, + ) -> ActionResult[CheckRun]: ... + + +@runtime_checkable +class UpdateCheckRunProtocol(Protocol): + def update_check_run( + self, + check_run_id: ResourceId, + status: BuildStatus | None = None, + conclusion: BuildConclusion | None = None, + output: CheckRunOutput | None = None, + ) -> ActionResult[CheckRun]: ... + + +# Review Protocols + + +@runtime_checkable +class CreateReviewCommentFileProtocol(Protocol): + def create_review_comment_file( + self, + pull_request_id: str, + commit_id: SHA, + body: str, + path: str, + side: ReviewSide, + ) -> ActionResult[ReviewComment]: ... + + +@runtime_checkable +class CreateReviewCommentLineProtocol(Protocol): + def create_review_comment_line( + self, + pull_request_id: str, + commit_id: SHA, + body: str, + path: str, + line: int, + side: ReviewSide, + ) -> ActionResult[ReviewComment]: ... + + +@runtime_checkable +class CreateReviewCommentMultilineProtocol(Protocol): + def create_review_comment_multiline( + self, + pull_request_id: str, + commit_id: SHA, + body: str, + path: str, + start_line: int, + start_side: ReviewSide, + end_line: int, + end_side: ReviewSide, + ) -> ActionResult[ReviewComment]: ... + + +@runtime_checkable +class CreateReviewCommentReplyProtocol(Protocol): + def create_review_comment_reply( + self, + pull_request_id: str, + body: str, + comment_id: str, + ) -> ActionResult[ReviewComment]: ... + + +@runtime_checkable +class CreateReviewProtocol(Protocol): + def create_review( + self, + pull_request_id: str, + commit_sha: SHA, + event: ReviewEvent, + comments: list[ReviewCommentInput], + body: str | None = None, + ) -> ActionResult[Review]: ... + + +# Moderation Protocols + + +@runtime_checkable +class MinimizeCommentProtocol(Protocol): + def minimize_comment(self, comment_node_id: str, reason: str) -> None: ... + + +@runtime_checkable +class ResolveReviewThreadProtocol(Protocol): + def resolve_review_thread(self, thread_node_id: str) -> None: ... + + +ALL_PROTOCOLS = ( + CompareCommitsProtocol, + CreateBranchProtocol, + CreateCheckRunProtocol, + CreateGitBlobProtocol, + CreateGitCommitProtocol, + CreateGitTreeProtocol, + CreateIssueCommentProtocol, + CreateIssueCommentReactionProtocol, + CreateIssueReactionProtocol, + CreatePullRequestCommentProtocol, + CreatePullRequestCommentReactionProtocol, + CreatePullRequestDraftProtocol, + CreatePullRequestProtocol, + CreatePullRequestReactionProtocol, + CreateReviewCommentFileProtocol, + CreateReviewCommentLineProtocol, + CreateReviewCommentMultilineProtocol, + CreateReviewCommentReplyProtocol, + CreateReviewProtocol, + DeleteIssueCommentProtocol, + DeleteIssueCommentReactionProtocol, + DeleteIssueReactionProtocol, + DeletePullRequestCommentProtocol, + DeletePullRequestCommentReactionProtocol, + DeletePullRequestReactionProtocol, + GetArchiveLinkProtocol, + GetBranchProtocol, + GetCheckRunProtocol, + GetCommitProtocol, + GetCommitsByPathProtocol, + GetCommitsProtocol, + GetFileContentProtocol, + GetGitCommitProtocol, + GetIssueCommentReactionsProtocol, + GetIssueCommentsProtocol, + GetIssueReactionsProtocol, + GetPullRequestCommentReactionsProtocol, + GetPullRequestCommentsProtocol, + GetPullRequestCommitsProtocol, + GetPullRequestDiffProtocol, + GetPullRequestFilesProtocol, + GetPullRequestProtocol, + GetPullRequestReactionsProtocol, + GetPullRequestsProtocol, + GetTreeProtocol, + MinimizeCommentProtocol, + RequestReviewProtocol, + ResolveReviewThreadProtocol, + UpdateBranchProtocol, + UpdateCheckRunProtocol, + UpdatePullRequestProtocol, +) + + +class Provider(Protocol): + """ + Providers abstract over an integration. They map generic commands to service-provider specific + commands and they map the results of those commands to generic result-types. + + Providers necessarily offer a larger API surface than what is available in an integration. Some + methods may be duplicates in some providers. This is intentional. Providers capture programmer + intent and translate it into a concrete interface. Therefore, providers provide a large range + of behaviors which may or may not be explicitly defined on a service-provider. + + Providers, also by necessity, offer a smaller API surface than what the SCM platform can + maximally provide. There are simply some operations which can not be adequately translated + between providers. None the less, we want to have a service-agnostic interface. This problem + is solved with capability-object-like system. Capabilities are progressively opted into using + structural sub-typing. As a provider's surface area expands the SourceCodeManager class will + automatically recognize that the provider has a particular capability and return "true" when + handling "can" requests. + """ + + organization_id: int + repository: Repository + + def is_rate_limited(self, referrer: Referrer) -> bool: ... diff --git a/tests/sentry/scm/test_fixtures.py b/tests/sentry/scm/test_fixtures.py index 9c4532b554f45d..191632a30daf90 100644 --- a/tests/sentry/scm/test_fixtures.py +++ b/tests/sentry/scm/test_fixtures.py @@ -4,7 +4,6 @@ from sentry.integrations.github.client import GitHubApiClient, GitHubReaction from sentry.integrations.models import Integration -from sentry.scm.private.provider import Provider from sentry.scm.types import ( ActionResult, BuildConclusion, @@ -25,6 +24,7 @@ PaginatedActionResult, PaginatedResponseMeta, PaginationParams, + Provider, PullRequest, PullRequestBranch, PullRequestCommit, diff --git a/tests/sentry/scm/unit/test_scm_actions.py b/tests/sentry/scm/unit/test_scm_actions.py index f4dd319643629b..cbbb2b231f0b60 100644 --- a/tests/sentry/scm/unit/test_scm_actions.py +++ b/tests/sentry/scm/unit/test_scm_actions.py @@ -60,8 +60,14 @@ SCMProviderException, SCMUnhandledException, ) -from sentry.scm.private.provider import GetBranchProtocol, GetIssueReactionsProtocol -from sentry.scm.types import PaginatedActionResult, ReactionResult, Referrer, Repository +from sentry.scm.types import ( + GetBranchProtocol, + GetIssueReactionsProtocol, + PaginatedActionResult, + ReactionResult, + Referrer, + Repository, +) from tests.sentry.scm.test_fixtures import BaseTestProvider From bf8ab3a6a5cc664f64fddcc4aa27a2bd30064f5d Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 27 Mar 2026 10:22:45 -0400 Subject: [PATCH 05/13] feat(replays): Make bulk delete API endpoints public (#111679) Change publish_status from PRIVATE to PUBLIC on the replay deletion job endpoints (index and detail) and add @extend_schema decorators with operation IDs, response types, and OpenAPI examples so that drf-spectacular generates documentation for them. Co-Authored-By: Claude Opus 4.6 --- .../apidocs/examples/replay_examples.py | 65 +++++++++++++++ src/sentry/apidocs/parameters.py | 8 ++ .../endpoints/project_replay_jobs_delete.py | 83 ++++++++++++++++++- 3 files changed, 153 insertions(+), 3 deletions(-) diff --git a/src/sentry/apidocs/examples/replay_examples.py b/src/sentry/apidocs/examples/replay_examples.py index 7dc96ad4888b9e..f3950639384a70 100644 --- a/src/sentry/apidocs/examples/replay_examples.py +++ b/src/sentry/apidocs/examples/replay_examples.py @@ -200,6 +200,71 @@ class ReplayExamples: ) ] + GET_REPLAY_DELETION_JOBS = [ + OpenApiExample( + "List replay deletion jobs", + value={ + "data": [ + { + "id": 1, + "dateCreated": "2024-01-01T00:00:00Z", + "dateUpdated": "2024-01-01T00:05:00Z", + "rangeStart": "2023-12-01T00:00:00Z", + "rangeEnd": "2024-01-01T00:00:00Z", + "environments": ["production"], + "status": "pending", + "query": "user.email:test@example.com", + "countDeleted": 0, + } + ] + }, + status_codes=["200"], + response_only=True, + ) + ] + + CREATE_REPLAY_DELETION_JOB = [ + OpenApiExample( + "Create a replay deletion job", + value={ + "data": { + "id": 1, + "dateCreated": "2024-01-01T00:00:00Z", + "dateUpdated": "2024-01-01T00:05:00Z", + "rangeStart": "2023-12-01T00:00:00Z", + "rangeEnd": "2024-01-01T00:00:00Z", + "environments": ["production"], + "status": "pending", + "query": "user.email:test@example.com", + "countDeleted": 0, + } + }, + status_codes=["201"], + response_only=True, + ) + ] + + GET_REPLAY_DELETION_JOB = [ + OpenApiExample( + "Get a replay deletion job", + value={ + "data": { + "id": 1, + "dateCreated": "2024-01-01T00:00:00Z", + "dateUpdated": "2024-01-01T00:05:00Z", + "rangeStart": "2023-12-01T00:00:00Z", + "rangeEnd": "2024-01-01T00:00:00Z", + "environments": ["production"], + "status": "pending", + "query": "user.email:test@example.com", + "countDeleted": 0, + } + }, + status_codes=["200"], + response_only=True, + ) + ] + GET_REPLAY_VIEWED_BY = [ OpenApiExample( "Get list of users who have viewed a replay", diff --git a/src/sentry/apidocs/parameters.py b/src/sentry/apidocs/parameters.py index bebff27aef1a10..2cbb90a209ba93 100644 --- a/src/sentry/apidocs/parameters.py +++ b/src/sentry/apidocs/parameters.py @@ -927,6 +927,14 @@ class ReplayParams: description="""The ID of the segment you'd like to retrieve.""", ) + JOB_ID = OpenApiParameter( + name="job_id", + location="path", + required=True, + type=OpenApiTypes.INT, + description="""The ID of the replay deletion job you'd like to retrieve.""", + ) + class NotificationParams: TRIGGER_TYPE = OpenApiParameter( diff --git a/src/sentry/replays/endpoints/project_replay_jobs_delete.py b/src/sentry/replays/endpoints/project_replay_jobs_delete.py index 18239a97f386ed..9326af6caf8991 100644 --- a/src/sentry/replays/endpoints/project_replay_jobs_delete.py +++ b/src/sentry/replays/endpoints/project_replay_jobs_delete.py @@ -1,3 +1,8 @@ +from __future__ import annotations + +from typing import TypedDict + +from drf_spectacular.utils import extend_schema from rest_framework import serializers from rest_framework.request import Request from rest_framework.response import Response @@ -10,12 +15,36 @@ from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.paginator import OffsetPaginator from sentry.api.serializers import Serializer, serialize +from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND +from sentry.apidocs.examples.replay_examples import ReplayExamples +from sentry.apidocs.parameters import GlobalParams, ReplayParams +from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.models import ReplayDeletionJobModel from sentry.replays.permissions import has_replay_permission from sentry.replays.tasks import run_bulk_replay_delete_job +class ReplayDeletionJobResponseData(TypedDict): + id: int + dateCreated: str + dateUpdated: str + rangeStart: str + rangeEnd: str + environments: list[str] + status: str + query: str + countDeleted: int + + +class ReplayDeletionJobListResponse(TypedDict): + data: list[ReplayDeletionJobResponseData] + + +class ReplayDeletionJobDetailResponse(TypedDict): + data: ReplayDeletionJobResponseData + + class ReplayDeletionJobPermission(ProjectPermission): scope_map = { "GET": ["project:write", "project:admin"], @@ -57,14 +86,29 @@ class ReplayDeletionJobCreateSerializer(serializers.Serializer): @cell_silo_endpoint +@extend_schema(tags=["Replays"]) class ProjectReplayDeletionJobsIndexEndpoint(ProjectEndpoint): owner = ApiOwner.DATA_BROWSING publish_status = { - "GET": ApiPublishStatus.PRIVATE, - "POST": ApiPublishStatus.PRIVATE, + "GET": ApiPublishStatus.PUBLIC, + "POST": ApiPublishStatus.PUBLIC, } permission_classes = (ReplayDeletionJobPermission,) + @extend_schema( + operation_id="List Replay Deletion Jobs", + parameters=[ + GlobalParams.ORG_ID_OR_SLUG, + GlobalParams.PROJECT_ID_OR_SLUG, + ], + responses={ + 200: inline_sentry_response_serializer( + "ListReplayDeletionJobs", ReplayDeletionJobListResponse + ), + 403: RESPONSE_FORBIDDEN, + }, + examples=ReplayExamples.GET_REPLAY_DELETION_JOBS, + ) def get(self, request: Request, project) -> Response: """ Retrieve a collection of replay delete jobs. @@ -86,6 +130,22 @@ def get(self, request: Request, project) -> Response: paginator_cls=OffsetPaginator, ) + @extend_schema( + operation_id="Create a Replay Deletion Job", + parameters=[ + GlobalParams.ORG_ID_OR_SLUG, + GlobalParams.PROJECT_ID_OR_SLUG, + ], + request=ReplayDeletionJobCreateSerializer, + responses={ + 201: inline_sentry_response_serializer( + "CreateReplayDeletionJob", ReplayDeletionJobDetailResponse + ), + 400: RESPONSE_BAD_REQUEST, + 403: RESPONSE_FORBIDDEN, + }, + examples=ReplayExamples.CREATE_REPLAY_DELETION_JOB, + ) def post(self, request: Request, project) -> Response: """ Create a new replay deletion job. @@ -132,12 +192,29 @@ def post(self, request: Request, project) -> Response: @cell_silo_endpoint +@extend_schema(tags=["Replays"]) class ProjectReplayDeletionJobDetailEndpoint(ProjectReplayEndpoint): publish_status = { - "GET": ApiPublishStatus.PRIVATE, + "GET": ApiPublishStatus.PUBLIC, } permission_classes = (ReplayDeletionJobPermission,) + @extend_schema( + operation_id="Get a Replay Deletion Job", + parameters=[ + GlobalParams.ORG_ID_OR_SLUG, + GlobalParams.PROJECT_ID_OR_SLUG, + ReplayParams.JOB_ID, + ], + responses={ + 200: inline_sentry_response_serializer( + "GetReplayDeletionJob", ReplayDeletionJobDetailResponse + ), + 403: RESPONSE_FORBIDDEN, + 404: RESPONSE_NOT_FOUND, + }, + examples=ReplayExamples.GET_REPLAY_DELETION_JOB, + ) def get(self, request: Request, project, job_id: int) -> Response: """ Fetch a replay delete job instance. From 0afffff965ca3b1ced66421909c0530f4187e70a Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Fri, 27 Mar 2026 10:48:01 -0400 Subject: [PATCH 06/13] fix(dashboards): Copy page filters when duplicating prebuilt dashboards (#111534) When duplicating a prebuilt dashboard, the clone was reconstructed from static config which has empty default values for page-level filters. The previous fix (#111017) copied the `DashboardFilters` object (`globalFilter`, `release`) but missed the top-level page filter fields (`projects`, `environment`, `period`, `start`, `end`, `utc`). This also fixes the manage/list view duplication path (`useDuplicateDashboard`) which never fetched the saved dashboard instance at all for prebuilt dashboards, so all saved filters (including `globalFilter`) were lost in that code path. Both paths now use a shared `copySavedFilters` helper that copies all 7 filter fields from the saved instance onto the clone. Refs DAIN-1406 Co-authored-by: Claude --- .../hooks/useDuplicateDashboard.spec.tsx | 80 ++++++++++++++++++- .../hooks/useDuplicateDashboard.tsx | 43 ++++++++-- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/static/app/views/dashboards/hooks/useDuplicateDashboard.spec.tsx b/static/app/views/dashboards/hooks/useDuplicateDashboard.spec.tsx index 1bea2595c12150..ed4efde8da82e1 100644 --- a/static/app/views/dashboards/hooks/useDuplicateDashboard.spec.tsx +++ b/static/app/views/dashboards/hooks/useDuplicateDashboard.spec.tsx @@ -79,6 +79,74 @@ describe('useDuplicateDashboard', () => { expect(createMock).toHaveBeenCalled(); expect(onSuccess).toHaveBeenCalled(); }); + + it('copies saved filters and page filters when duplicating a prebuilt dashboard', async () => { + const savedFilters: DashboardFilters = { + globalFilter: [ + { + dataset: WidgetType.SPANS, + tag: {key: 'span.system', name: 'span.system'}, + value: 'postgresql', + }, + ], + }; + // Mock for fetchDashboard (saved instance with user's filters) + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/dashboards/55/`, + body: DashboardFixture([], { + id: '55', + prebuiltId: PrebuiltDashboardId.BACKEND_QUERIES, + filters: savedFilters, + projects: [1, 2], + environment: ['production'], + period: '7d', + }), + }); + // Mock for resolveLinkedDashboardIds (resolves linked summary dashboard) + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/dashboards/`, + method: 'GET', + body: [ + DashboardFixture([], { + id: '56', + prebuiltId: PrebuiltDashboardId.BACKEND_QUERIES_SUMMARY, + }), + ], + }); + const createMock = MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/dashboards/`, + method: 'POST', + body: DashboardFixture([], {id: '200'}), + }); + + const onSuccess = jest.fn(); + const {result} = renderHookWithProviders(() => useDuplicateDashboard({onSuccess}), { + organization, + }); + + await act(async () => { + await result.current( + DashboardListItemFixture({ + id: '55', + prebuiltId: PrebuiltDashboardId.BACKEND_QUERIES, + }), + 'grid' + ); + }); + + expect(createMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + data: expect.objectContaining({ + filters: savedFilters, + projects: [1, 2], + environment: ['production'], + period: '7d', + }), + }) + ); + expect(onSuccess).toHaveBeenCalled(); + }); }); describe('useDuplicatePrebuiltDashboard', () => { @@ -88,7 +156,7 @@ describe('useDuplicatePrebuiltDashboard', () => { MockApiClient.clearMockResponses(); }); - it('fetches saved dashboard details and duplicates with saved filters', async () => { + it('fetches saved dashboard details and duplicates with saved filters and page filters', async () => { const savedFilters: DashboardFilters = { globalFilter: [ { @@ -104,6 +172,9 @@ describe('useDuplicatePrebuiltDashboard', () => { id: '55', prebuiltId: PrebuiltDashboardId.BACKEND_QUERIES_SUMMARY, filters: savedFilters, + projects: [3, 4], + environment: ['staging'], + period: '14d', }), }); const createMock = MockApiClient.addMockResponse({ @@ -125,7 +196,12 @@ describe('useDuplicatePrebuiltDashboard', () => { expect(createMock).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - data: expect.objectContaining({filters: savedFilters}), + data: expect.objectContaining({ + filters: savedFilters, + projects: [3, 4], + environment: ['staging'], + period: '14d', + }), }) ); expect(onSuccess).toHaveBeenCalledWith(expect.objectContaining({id: '300'})); diff --git a/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx b/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx index 0f016fa3cdcc41..7bebd573c56b69 100644 --- a/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx +++ b/static/app/views/dashboards/hooks/useDuplicateDashboard.tsx @@ -27,13 +27,28 @@ export function useDuplicateDashboard({onSuccess}: UseDuplicateDashboardProps) { const duplicateDashboard = useCallback( async (dashboard: DashboardListItem, viewType: 'table' | 'grid') => { try { - const dashboardDetail = dashboard.prebuiltId - ? await resolveLinkedDashboardIds({ + let dashboardDetail: DashboardDetails; + if (dashboard.prebuiltId) { + // Widgets come from static config. If the dashboard has been saved + // (has a real ID), also fetch the saved instance to copy its filters. + const hasSavedInstance = dashboard.id && dashboard.id !== '-1'; + const [resolved, saved] = await Promise.all([ + resolveLinkedDashboardIds({ queryClient, orgSlug: organization.slug, dashboard: toPrebuiltDashboardDetails(dashboard.prebuiltId), - }) - : await fetchDashboard(api, organization.slug, dashboard.id); + }), + hasSavedInstance + ? fetchDashboard(api, organization.slug, dashboard.id) + : Promise.resolve(undefined), + ]); + dashboardDetail = resolved; + if (saved) { + copySavedFilters(dashboardDetail, saved); + } + } else { + dashboardDetail = await fetchDashboard(api, organization.slug, dashboard.id); + } const newDashboard = cloneDashboard(dashboardDetail); newDashboard.title = `${newDashboard.title} copy`; @@ -93,9 +108,7 @@ export function useDuplicatePrebuiltDashboard({onSuccess}: UseDuplicateDashboard delete newDashboard.prebuiltId; newDashboard.title = `${newDashboard.title} copy`; newDashboard.widgets.map(widget => (widget.id = undefined)); - if (savedDashboard.filters !== undefined) { - newDashboard.filters = savedDashboard.filters; - } + copySavedFilters(newDashboard, savedDashboard); const copiedDashboard = await createDashboard( api, organization.slug, @@ -115,6 +128,22 @@ export function useDuplicatePrebuiltDashboard({onSuccess}: UseDuplicateDashboard return {duplicatePrebuiltDashboard, isLoading}; } +/** + * Copies saved filter state from a persisted dashboard onto a target. + * This includes both the DashboardFilters object (globalFilter, release) and + * the page-level filters (projects, environment, date range) which are stored + * as top-level fields. + */ +function copySavedFilters(target: DashboardDetails, source: DashboardDetails): void { + target.filters = source.filters; + target.projects = source.projects; + target.environment = source.environment; + target.period = source.period; + target.start = source.start; + target.end = source.end; + target.utc = source.utc; +} + /** * Prebuilt dashboard configs don't have an `id` since they aren't persisted. * `cloneDashboard` and `createDashboard` require `DashboardDetails` (which includes `id`), From 352c3c943e3bbade173fbbd3702cafcf9ba40913 Mon Sep 17 00:00:00 2001 From: Lyn Nagara <1779792+lynnagara@users.noreply.github.com> Date: Fri, 27 Mar 2026 07:51:40 -0700 Subject: [PATCH 07/13] ref(cells): Update silo mode test decorators to pass cell (#111602) region is being deprecated from this decorator, switch to the new parameter `cells` --- .agents/skills/hybrid-cloud-rpc/SKILL.md | 16 ++++----- .agents/skills/hybrid-cloud-test-gen/SKILL.md | 32 ++++++++--------- .../references/api-gateway-tests.md | 22 ++++++------ .../references/endpoint-silo-tests.md | 16 ++++----- .../references/rpc-service-tests.md | 4 +-- src/sentry/testutils/helpers/apigateway.py | 14 ++++---- .../api/endpoints/test_organization_fork.py | 2 +- .../hybridcloud/apigateway/test_apigateway.py | 34 +++++++++---------- .../apigateway/test_apigateway_helpers.py | 2 +- .../hybridcloud/apigateway/test_proxy.py | 10 +++--- tests/sentry/hybridcloud/test_cell.py | 2 +- .../jira/test_sentry_issue_details.py | 2 +- tests/sentry/relocation/tasks/test_process.py | 2 +- .../sentry/relocation/tasks/test_transfer.py | 4 +-- tests/sentry/users/models/test_user.py | 2 +- 15 files changed, 82 insertions(+), 82 deletions(-) diff --git a/.agents/skills/hybrid-cloud-rpc/SKILL.md b/.agents/skills/hybrid-cloud-rpc/SKILL.md index 9b306a8ce766ae..1d883d872f388f 100644 --- a/.agents/skills/hybrid-cloud-rpc/SKILL.md +++ b/.agents/skills/hybrid-cloud-rpc/SKILL.md @@ -41,7 +41,7 @@ The service's `local_mode` determines where the database-backed implementation r | Data lives in... | `local_mode` | Decorator on methods | Example | | ------------------------------------------------- | ------------------ | ------------------------------- | ---------------------------------- | -| Region silo (projects, events, issues, org data) | `SiloMode.CELL` | `@cell_rpc_method(resolve=...)` | `OrganizationService` | +| Cell silo (projects, events, issues, org data) | `SiloMode.CELL` | `@cell_rpc_method(resolve=...)` | `OrganizationService` | | Control silo (users, auth, billing, org mappings) | `SiloMode.CONTROL` | `@rpc_method` | `OrganizationMemberMappingService` | **Decision rule**: If the Django models you need to query live in the cell database, use `SiloMode.CELL`. If they live in the control database, use `SiloMode.CONTROL`. @@ -95,7 +95,7 @@ If your service doesn't fit any of these, add a new entry to the `service_packag ## Step 4: Add or Update Methods -### For REGION silo services +### For CELL silo services Load `references/resolvers.md` for resolver details. @@ -217,11 +217,11 @@ Every RPC service needs three categories of tests: **silo mode compatibility**, ### 7.1 Silo mode compatibility with `@all_silo_test` -Every service test class MUST use `@all_silo_test` so tests run in all three modes (MONOLITH, REGION, CONTROL). This ensures the delegation layer works for both local and remote dispatch paths. +Every service test class MUST use `@all_silo_test` so tests run in all three modes (MONOLITH, CELL, CONTROL). This ensures the delegation layer works for both local and remote dispatch paths. ```python from sentry.testutils.cases import TestCase, TransactionTestCase -from sentry.testutils.silo import all_silo_test, assume_test_silo_mode, create_test_regions +from sentry.testutils.silo import all_silo_test, assume_test_silo_mode, create_test_cells @all_silo_test class MyServiceTest(TestCase): @@ -234,8 +234,8 @@ class MyServiceTest(TestCase): For tests that need named cells (e.g., testing cell resolution): ```python -@all_silo_test(regions=create_test_regions("us", "eu")) -class MyServiceRegionTest(TransactionTestCase): +@all_silo_test(cells=create_test_cells("us", "eu")) +class MyServiceCellTest(TransactionTestCase): ... ``` @@ -403,7 +403,7 @@ from sentry.testutils.silo import ( cell_silo_test, assume_test_silo_mode, assume_test_silo_mode_of, - create_test_regions, + create_test_cells, ) from sentry.testutils.outbox import outbox_runner from sentry.testutils.hybrid_cloud import HybridCloudTestMixin @@ -423,7 +423,7 @@ Before submitting your PR, verify: - [ ] All RPC method parameters are keyword-only (`*` separator) - [ ] All parameters have explicit type annotations - [ ] All types are serializable (primitives, RpcModel, list, Optional, dict, Enum, datetime) -- [ ] Region service methods have `@cell_rpc_method` with appropriate resolver +- [ ] Cell service methods have `@cell_rpc_method` with appropriate resolver - [ ] Control service methods have `@rpc_method` - [ ] `@cell_rpc_method` / `@rpc_method` comes BEFORE `@abstractmethod` - [ ] `create_delegation()` is called at module level at the bottom of service.py diff --git a/.agents/skills/hybrid-cloud-test-gen/SKILL.md b/.agents/skills/hybrid-cloud-test-gen/SKILL.md index 95a2bccdb48d66..6507a24eeb3ecb 100644 --- a/.agents/skills/hybrid-cloud-test-gen/SKILL.md +++ b/.agents/skills/hybrid-cloud-test-gen/SKILL.md @@ -65,17 +65,17 @@ RPC service tests must cover: ### Quick Reference — Decorator & Base Class -| Scenario | Decorator | Base Class | -| ---------------------------------- | --------------------------------------------------- | -------------------------------- | -| Standard RPC service | `@all_silo_test` | `TestCase` | -| RPC with named regions | `@all_silo_test(regions=create_test_regions("us"))` | `TestCase` | -| RPC with member mapping assertions | `@all_silo_test` | `TestCase, HybridCloudTestMixin` | +| Scenario | Decorator | Base Class | +| ---------------------------------- | ----------------------------------------------- | -------------------------------- | +| Standard RPC service | `@all_silo_test` | `TestCase` | +| RPC with named cells | `@all_silo_test(cells=create_test_cells("us"))` | `TestCase` | +| RPC with member mapping assertions | `@all_silo_test` | `TestCase, HybridCloudTestMixin` | ## Step 4: Generate API Gateway Tests Load `references/api-gateway-tests.md` for complete templates and patterns. -API gateway tests verify that requests to control-silo endpoints are correctly proxied to the appropriate region. They must cover: +API gateway tests verify that requests to control-silo endpoints are correctly proxied to the appropriate cell. They must cover: - **Proxy pass-through**: Requests forwarded with correct params, headers, body - **Query parameter forwarding**: Multi-value params preserved @@ -84,9 +84,9 @@ API gateway tests verify that requests to control-silo endpoints are correctly p ### Quick Reference — Decorator & Base Class -| Scenario | Decorator | Base Class | -| --------------------- | ------------------------------------------------------------------------------------ | -------------------- | -| Standard gateway test | `@control_silo_test(regions=[ApiGatewayTestCase.REGION], include_monolith_run=True)` | `ApiGatewayTestCase` | +| Scenario | Decorator | Base Class | +| --------------------- | -------------------------------------------------------------------------------- | -------------------- | +| Standard gateway test | `@control_silo_test(cells=[ApiGatewayTestCase.CELL], include_monolith_run=True)` | `ApiGatewayTestCase` | ## Step 5: Generate Outbox Pattern Tests @@ -120,12 +120,12 @@ Endpoint silo tests verify that API endpoints work correctly under their declare ### Quick Reference — Decorator Mapping -| Endpoint Decorator | Test Decorator | -| ------------------------------------- | ------------------------------------------------------- | -| `@cell_silo_endpoint ` | `@cell_silo_test` | -| `@control_silo_endpoint` | `@control_silo_test` | -| `@control_silo_endpoint` (with proxy) | `@control_silo_test(regions=create_test_regions("us"))` | -| No decorator (monolith-only) | `@no_silo_test` | +| Endpoint Decorator | Test Decorator | +| ------------------------------------- | --------------------------------------------------- | +| `@cell_silo_endpoint ` | `@cell_silo_test` | +| `@control_silo_endpoint` | `@control_silo_test` | +| `@control_silo_endpoint` (with proxy) | `@control_silo_test(cells=create_test_cells("us"))` | +| No decorator (monolith-only) | `@no_silo_test` | ## Step 7: Validate @@ -153,7 +153,7 @@ from sentry.testutils.silo import ( no_silo_test, assume_test_silo_mode, assume_test_silo_mode_of, - create_test_regions, + create_test_cells, ) # Base classes diff --git a/.agents/skills/hybrid-cloud-test-gen/references/api-gateway-tests.md b/.agents/skills/hybrid-cloud-test-gen/references/api-gateway-tests.md index 8344544f10e949..ea999d7508ede8 100644 --- a/.agents/skills/hybrid-cloud-test-gen/references/api-gateway-tests.md +++ b/.agents/skills/hybrid-cloud-test-gen/references/api-gateway-tests.md @@ -26,7 +26,7 @@ from sentry.utils import json ## Template: Standard API Gateway Test ```python -@control_silo_test(regions=[ApiGatewayTestCase.REGION], include_monolith_run=True) +@control_silo_test(cells=[ApiGatewayTestCase.CELL], include_monolith_run=True) class Test{Feature}ApiGateway(ApiGatewayTestCase): @responses.activate @@ -36,7 +36,7 @@ class Test{Feature}ApiGateway(ApiGatewayTestCase): headers = dict(example="this") responses.add_callback( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/{endpoint_path}/", + f"{self.CELL.address}/organizations/{self.organization.slug}/{endpoint_path}/", verify_request_params(query_params, headers), ) @@ -58,7 +58,7 @@ class Test{Feature}ApiGateway(ApiGatewayTestCase): headers = {"content-type": "application/json"} responses.add_callback( responses.POST, - f"{self.REGION.address}/organizations/{self.organization.slug}/{endpoint_path}/", + f"{self.CELL.address}/organizations/{self.organization.slug}/{endpoint_path}/", verify_request_body(request_body, headers), ) @@ -80,7 +80,7 @@ class Test{Feature}ApiGateway(ApiGatewayTestCase): """Verify upstream errors are forwarded to the client.""" responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/{endpoint_path}/", + f"{self.CELL.address}/organizations/{self.organization.slug}/{endpoint_path}/", status=400, json={"detail": "Bad request"}, ) @@ -104,7 +104,7 @@ In CONTROL mode, proxied responses are streamed. Use `close_streaming_response() """Verify proxied response content is correct.""" responses.add_callback( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/{endpoint_path}/", + f"{self.CELL.address}/organizations/{self.organization.slug}/{endpoint_path}/", verify_request_params({}, {}), ) @@ -128,7 +128,7 @@ In CONTROL mode, proxied responses are streamed. Use `close_streaming_response() ## Template: SiloLimit Availability Check ```python - def test_control_only_endpoint_unavailable_in_region(self): + def test_control_only_endpoint_unavailable_in_cell(self): """Verify control-only endpoints raise AvailabilityError outside their silo.""" with pytest.raises(SiloLimit.AvailabilityError): self.client.get("/api/0/{control-only-path}/") @@ -136,12 +136,12 @@ In CONTROL mode, proxied responses are streamed. Use `close_streaming_response() ## Key Patterns -- **`ApiGatewayTestCase`** sets up a test region, mock HTTP callbacks, and the API gateway middleware. It extends `APITestCase`. -- **`@control_silo_test(regions=[...], include_monolith_run=True)`** runs the test in both CONTROL and MONOLITH modes. -- **Every test method MUST use `@responses.activate`** because gateway tests mock HTTP calls to the region address. +- **`ApiGatewayTestCase`** sets up a test cell, mock HTTP callbacks, and the API gateway middleware. It extends `APITestCase`. +- **`@control_silo_test(cells=[...], include_monolith_run=True)`** runs the test in both CONTROL and MONOLITH modes. +- **Every test method MUST use `@responses.activate`** because gateway tests mock HTTP calls to the cell address. - **`verify_request_params(params, headers)`** is a callback that asserts query params and headers match. - **`verify_request_body(body, headers)`** asserts POST body matches. - **`close_streaming_response(resp)`** reads a streaming response to bytes — required for proxied responses in CONTROL mode. - **`override_settings(MIDDLEWARE=tuple(self.middleware))`** ensures the API gateway middleware is active. -- **`self.REGION`** is a pre-configured `Region` object with address `http://us.internal.sentry.io`. -- **`self.organization`** is pre-created in `setUp` and bound to `self.REGION`. +- **`self.CELL`** is a pre-configured `Cell` object with address `http://us.internal.sentry.io`. +- **`self.organization`** is pre-created in `setUp` and bound to `self.CELL`. diff --git a/.agents/skills/hybrid-cloud-test-gen/references/endpoint-silo-tests.md b/.agents/skills/hybrid-cloud-test-gen/references/endpoint-silo-tests.md index 17c76397c53cee..7ce8e2a8140735 100644 --- a/.agents/skills/hybrid-cloud-test-gen/references/endpoint-silo-tests.md +++ b/.agents/skills/hybrid-cloud-test-gen/references/endpoint-silo-tests.md @@ -10,7 +10,7 @@ from sentry.testutils.silo import ( no_silo_test, assume_test_silo_mode, assume_test_silo_mode_of, - create_test_regions, + create_test_cells, ) from sentry.silo.base import SiloMode ``` @@ -19,14 +19,14 @@ from sentry.silo.base import SiloMode Match the endpoint's silo decorator to the test's silo decorator: -| Endpoint Decorator | Test Decorator | -| -------------------------------------------- | ------------------------------------------------------- | -| `@cell_silo_endpoint` | `@cell_silo_test` | -| `@control_silo_endpoint` | `@control_silo_test` | -| `@control_silo_endpoint` (proxies to region) | `@control_silo_test(regions=create_test_regions("us"))` | -| No silo decorator | `@no_silo_test` | +| Endpoint Decorator | Test Decorator | +| ------------------------------------------ | --------------------------------------------------- | +| `@cell_silo_endpoint` | `@cell_silo_test` | +| `@control_silo_endpoint` | `@control_silo_test` | +| `@control_silo_endpoint` (proxies to cell) | `@control_silo_test(cells=create_test_cells("us"))` | +| No silo decorator | `@no_silo_test` | -## Template: Region Silo Endpoint Test +## Template: Cell Silo Endpoint Test ```python @cell_silo_test diff --git a/.agents/skills/hybrid-cloud-test-gen/references/rpc-service-tests.md b/.agents/skills/hybrid-cloud-test-gen/references/rpc-service-tests.md index 80d543697ca654..14aa8a04691123 100644 --- a/.agents/skills/hybrid-cloud-test-gen/references/rpc-service-tests.md +++ b/.agents/skills/hybrid-cloud-test-gen/references/rpc-service-tests.md @@ -27,7 +27,7 @@ from sentry.testutils.silo import ( all_silo_test, assume_test_silo_mode, assume_test_silo_mode_of, - create_test_regions, + create_test_cells, ) ``` @@ -92,7 +92,7 @@ class Test{ServiceName}Service(TestCase): Prefer `assume_test_silo_mode_of(Model)` over `assume_test_silo_mode(SiloMode.X)` when checking a single model: ```python -@all_silo_test(regions=create_test_regions("us")) +@all_silo_test(cells=create_test_cells("us")) class Test{ServiceName}CrossSilo(TestCase, HybridCloudTestMixin): def test_{method_name}_creates_mapping(self): with outbox_runner(): diff --git a/src/sentry/testutils/helpers/apigateway.py b/src/sentry/testutils/helpers/apigateway.py index a96cf527434104..08e3c4ac4bf6f1 100644 --- a/src/sentry/testutils/helpers/apigateway.py +++ b/src/sentry/testutils/helpers/apigateway.py @@ -137,9 +137,9 @@ def provision_middleware(): @override_settings(ROOT_URLCONF=__name__) class ApiGatewayTestCase(APITestCase): # Subclasses will generally need to be decorated with - # @*_silo_test(regions=[ApiGatewayTestCase.REGION]) + # @*_silo_test(cells=[ApiGatewayTestCase.CELL]) - REGION = Cell( + CELL = Cell( name="us", snowflake_id=1, address="http://us.internal.sentry.io", @@ -150,21 +150,21 @@ def setUp(self): super().setUp() responses.add( responses.GET, - f"{self.REGION.address}/get", + f"{self.CELL.address}/get", body=json.dumps({"proxy": True}), content_type="application/json", adding_headers={"test": "header"}, ) responses.add( responses.GET, - f"{self.REGION.address}/error", + f"{self.CELL.address}/error", body=json.dumps({"proxy": True}), status=400, content_type="application/json", adding_headers={"test": "header"}, ) - self.organization = self.create_organization(region=self.REGION) + self.organization = self.create_organization(region=self.CELL) # Echos the request body and header back for verification def return_request_body(request): @@ -175,7 +175,7 @@ def return_request_params(request): params = parse_qs(request.url.split("?")[1]) return (200, request.headers, json.dumps(params).encode()) - responses.add_callback(responses.GET, f"{self.REGION.address}/echo", return_request_params) - responses.add_callback(responses.POST, f"{self.REGION.address}/echo", return_request_body) + responses.add_callback(responses.GET, f"{self.CELL.address}/echo", return_request_params) + responses.add_callback(responses.POST, f"{self.CELL.address}/echo", return_request_body) self.middleware = provision_middleware() diff --git a/tests/sentry/api/endpoints/test_organization_fork.py b/tests/sentry/api/endpoints/test_organization_fork.py index d9d76fae630957..9953bdd548d4bc 100644 --- a/tests/sentry/api/endpoints/test_organization_fork.py +++ b/tests/sentry/api/endpoints/test_organization_fork.py @@ -25,7 +25,7 @@ @patch("sentry.analytics.record") @patch("sentry.relocation.tasks.process.uploading_start.apply_async") -@cell_silo_test(regions=SAAS_TO_SAAS_TEST_REGIONS) +@cell_silo_test(cells=SAAS_TO_SAAS_TEST_REGIONS) class OrganizationForkTest(APITestCase): endpoint = "sentry-api-0-organization-fork" method = "POST" diff --git a/tests/sentry/hybridcloud/apigateway/test_apigateway.py b/tests/sentry/hybridcloud/apigateway/test_apigateway.py index 992e91b217170f..723305c5ab8568 100644 --- a/tests/sentry/hybridcloud/apigateway/test_apigateway.py +++ b/tests/sentry/hybridcloud/apigateway/test_apigateway.py @@ -14,7 +14,7 @@ from sentry.utils import json -@control_silo_test(cells=[ApiGatewayTestCase.REGION], include_monolith_run=True) +@control_silo_test(cells=[ApiGatewayTestCase.CELL], include_monolith_run=True) class ApiGatewayTest(ApiGatewayTestCase): @responses.activate def test_simple(self) -> None: @@ -22,7 +22,7 @@ def test_simple(self) -> None: headers = dict(example="this") responses.add_callback( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/region/", + f"{self.CELL.address}/organizations/{self.organization.slug}/region/", verify_request_params(query_params, headers), ) @@ -44,7 +44,7 @@ def test_simple(self) -> None: def test_proxy_does_not_resolve_redirect(self) -> None: responses.add( responses.POST, - f"{self.REGION.address}/organizations/{self.organization.slug}/region/", + f"{self.CELL.address}/organizations/{self.organization.slug}/region/", headers={"Location": "https://zombo.com"}, status=302, ) @@ -78,12 +78,12 @@ def test_proxy_check_org_slug_url(self) -> None: """Test the logic of when a request should be proxied""" responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/region/", + f"{self.CELL.address}/organizations/{self.organization.slug}/region/", json={"proxy": True}, ) responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/control/", + f"{self.CELL.address}/organizations/{self.organization.slug}/control/", json={"proxy": True}, ) @@ -114,22 +114,22 @@ def test_proxy_check_org_id_or_slug_url_with_params(self) -> None: """Test the logic of when a request should be proxied""" responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/region/", + f"{self.CELL.address}/organizations/{self.organization.slug}/region/", json={"proxy": True}, ) responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.slug}/control/", + f"{self.CELL.address}/organizations/{self.organization.slug}/control/", json={"proxy": True}, ) responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.id}/region/", + f"{self.CELL.address}/organizations/{self.organization.id}/region/", json={"proxy": True}, ) responses.add( responses.GET, - f"{self.REGION.address}/organizations/{self.organization.id}/control/", + f"{self.CELL.address}/organizations/{self.organization.id}/control/", json={"proxy": True}, ) @@ -183,7 +183,7 @@ def test_proxy_check_region_pinned_url(self) -> None: project_key = self.create_project_key(self.project) responses.add( responses.GET, - f"{self.REGION.address}/js-sdk-loader/{project_key.public_key}.js", + f"{self.CELL.address}/js-sdk-loader/{project_key.public_key}.js", json={"proxy": True}, ) @@ -206,15 +206,15 @@ def test_proxy_check_region_pinned_url(self) -> None: assert resp.data["proxy"] is False @responses.activate - def test_proxy_check_region_pinned_url_with_params(self) -> None: + def test_proxy_check_cell_pinned_url_with_params(self) -> None: responses.add( responses.GET, - f"{self.REGION.address}/relays/register/", + f"{self.CELL.address}/relays/register/", json={"proxy": True}, ) responses.add( responses.GET, - f"{self.REGION.address}/relays/abc123/", + f"{self.CELL.address}/relays/abc123/", json={"proxy": True, "details": True}, ) @@ -231,16 +231,16 @@ def test_proxy_check_region_pinned_url_with_params(self) -> None: assert resp_json["details"] is True @responses.activate - def test_proxy_check_region_pinned_issue_urls(self) -> None: + def test_proxy_check_cell_pinned_issue_urls(self) -> None: issue = self.create_group() responses.add( responses.GET, - f"{self.REGION.address}/issues/{issue.id}/", + f"{self.CELL.address}/issues/{issue.id}/", json={"proxy": True, "id": issue.id}, ) responses.add( responses.GET, - f"{self.REGION.address}/issues/{issue.id}/events/", + f"{self.CELL.address}/issues/{issue.id}/events/", json={"proxy": True, "id": issue.id, "events": True}, ) @@ -266,7 +266,7 @@ def test_proxy_check_region_pinned_issue_urls(self) -> None: def test_proxy_error_embed_dsn(self) -> None: responses.add( responses.GET, - f"{self.REGION.address}/api/embed/error-page/", + f"{self.CELL.address}/api/embed/error-page/", json={"proxy": True, "name": "error-embed"}, ) with override_settings(SILO_MODE=SiloMode.CONTROL, MIDDLEWARE=tuple(self.middleware)): diff --git a/tests/sentry/hybridcloud/apigateway/test_apigateway_helpers.py b/tests/sentry/hybridcloud/apigateway/test_apigateway_helpers.py index 9eca3bec73ee23..f07005d6d73707 100644 --- a/tests/sentry/hybridcloud/apigateway/test_apigateway_helpers.py +++ b/tests/sentry/hybridcloud/apigateway/test_apigateway_helpers.py @@ -8,7 +8,7 @@ from sentry.utils import json -@no_silo_test(cells=[ApiGatewayTestCase.REGION]) +@no_silo_test(cells=[ApiGatewayTestCase.CELL]) class VerifyRequestBodyTest(ApiGatewayTestCase): @responses.activate def test_verify_request_body(self) -> None: diff --git a/tests/sentry/hybridcloud/apigateway/test_proxy.py b/tests/sentry/hybridcloud/apigateway/test_proxy.py index 47e5913de61139..c1458db1ba4f71 100644 --- a/tests/sentry/hybridcloud/apigateway/test_proxy.py +++ b/tests/sentry/hybridcloud/apigateway/test_proxy.py @@ -30,7 +30,7 @@ url_name = "sentry-api-0-projets" -@control_silo_test(cells=[ApiGatewayTestCase.REGION], include_monolith_run=True) +@control_silo_test(cells=[ApiGatewayTestCase.CELL], include_monolith_run=True) class ProxyTestCase(ApiGatewayTestCase): @responses.activate def test_simple(self) -> None: @@ -275,7 +275,7 @@ def test_strip_request_headers(self) -> None: } -@control_silo_test(regions=[ApiGatewayTestCase.REGION]) +@control_silo_test(cells=[ApiGatewayTestCase.CELL]) class ProxyCircuitBreakerTestCase(ApiGatewayTestCase): def _make_breaker_mock(self, *, allow_request: bool) -> MagicMock: mock_breaker = MagicMock() @@ -304,7 +304,7 @@ def test_circuit_breaker_keyed_per_cell(self) -> None: request = RequestFactory().get("http://sentry.io/get") proxy_request(request, self.organization.slug, url_name) key_used = mock_breaker_class.call_args.kwargs["key"] - assert key_used == f"apigateway.proxy.{self.REGION.name}" + assert key_used == f"apigateway.proxy.{self.CELL.name}" @responses.activate def test_circuit_breaker_disabled_by_default(self) -> None: @@ -339,7 +339,7 @@ def test_handles_invalid_config(self) -> None: def test_timeout_records_error(self) -> None: responses.add( responses.GET, - f"{self.REGION.address}/timeout", + f"{self.CELL.address}/timeout", body=Timeout(), ) with patch("sentry.hybridcloud.apigateway.proxy.CircuitBreaker") as mock_breaker_class: @@ -355,7 +355,7 @@ def test_timeout_records_error(self) -> None: def test_5xx_response_records_error(self) -> None: responses.add( responses.GET, - f"{self.REGION.address}/server-error", + f"{self.CELL.address}/server-error", status=500, body=json.dumps({"detail": "internal server error"}), content_type="application/json", diff --git a/tests/sentry/hybridcloud/test_cell.py b/tests/sentry/hybridcloud/test_cell.py index 3d652f0e0f633c..a1ddb07cc44b3f 100644 --- a/tests/sentry/hybridcloud/test_cell.py +++ b/tests/sentry/hybridcloud/test_cell.py @@ -21,7 +21,7 @@ ) -@control_silo_test(regions=_TEST_CELLS) +@control_silo_test(cells=_TEST_CELLS) class CellResolutionTest(TestCase): def setUp(self) -> None: self.target_cell = _TEST_CELLS[0] diff --git a/tests/sentry/integrations/jira/test_sentry_issue_details.py b/tests/sentry/integrations/jira/test_sentry_issue_details.py index 0db8dcddb54b95..7ccd484f9e2484 100644 --- a/tests/sentry/integrations/jira/test_sentry_issue_details.py +++ b/tests/sentry/integrations/jira/test_sentry_issue_details.py @@ -173,7 +173,7 @@ def test_simple_not_linked(self, mock_get_integration_from_request: MagicMock) - assert b"This Sentry issue is not linked to a Jira issue" in response.content -@control_silo_test(regions=region_config) +@control_silo_test(cells=region_config) class JiraIssueHookControlTest(APITestCase): def setUp(self) -> None: super().setUp() diff --git a/tests/sentry/relocation/tasks/test_process.py b/tests/sentry/relocation/tasks/test_process.py index 454defcbc67dfb..8fa4e5bd7a0d58 100644 --- a/tests/sentry/relocation/tasks/test_process.py +++ b/tests/sentry/relocation/tasks/test_process.py @@ -223,7 +223,7 @@ def mock_message_builder(self, fake_message_builder: Mock): @patch("sentry.backup.crypto.KeyManagementServiceClient") @patch("sentry.relocation.utils.MessageBuilder") @patch("sentry.relocation.tasks.process.uploading_complete.apply_async") -@cell_silo_test(regions=SAAS_TO_SAAS_TEST_REGIONS) +@cell_silo_test(cells=SAAS_TO_SAAS_TEST_REGIONS) class UploadingStartTest(RelocationTaskTestCase): def setUp(self) -> None: self.owner = self.create_user( diff --git a/tests/sentry/relocation/tasks/test_transfer.py b/tests/sentry/relocation/tasks/test_transfer.py index ec94f7692645f7..01b51a0a54d364 100644 --- a/tests/sentry/relocation/tasks/test_transfer.py +++ b/tests/sentry/relocation/tasks/test_transfer.py @@ -130,7 +130,7 @@ def test_purge_expired(self, mock_process: MagicMock) -> None: assert not RegionRelocationTransfer.objects.filter(id=transfer.id).exists() -@control_silo_test(regions=TEST_REGIONS) +@control_silo_test(cells=TEST_REGIONS) class ProcessRelocationTransferControlTest(TestCase): def test_missing_transfer(self) -> None: res = process_relocation_transfer_control(transfer_id=999) @@ -181,7 +181,7 @@ def test_transfer_reply_state(self, mock_uploading_complete: MagicMock) -> None: assert RelocationFile.objects.filter(relocation=relocation).exists() -@cell_silo_test(regions=TEST_REGIONS) +@cell_silo_test(cells=TEST_REGIONS) class ProcessRelocationTransferRegionTest(TestCase): def test_missing_transfer(self) -> None: res = process_relocation_transfer_region(transfer_id=999) diff --git a/tests/sentry/users/models/test_user.py b/tests/sentry/users/models/test_user.py index 5f9e837f9a18b3..a34ab72712ae1b 100644 --- a/tests/sentry/users/models/test_user.py +++ b/tests/sentry/users/models/test_user.py @@ -52,7 +52,7 @@ ) -@control_silo_test(regions=_TEST_CELLS) +@control_silo_test(cells=_TEST_CELLS) class UserHybridCloudDeletionTest(TestCase): def setUp(self) -> None: super().setUp() From d4c27af18ff157e3bb46ca8dbfe94228a3e252ab Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:07:53 -0400 Subject: [PATCH 08/13] fix(dashboards): Hide prebuilt dashboards from All Dashboards list (#111709) Hide Sentry Built dashboards from the All Dashboards list by adding the `excludePrebuilt` filter to the dashboard list API call. Previously the All Dashboards view showed all dashboards including prebuilt ones, which was surfaced as feedback from both internal and external users. Prebuilt dashboards remain discoverable via the Sentry Built tab. Refs LINEAR-DAIN-1415 Co-authored-by: Claude Opus 4.6 --- static/app/views/dashboards/manage/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/static/app/views/dashboards/manage/index.tsx b/static/app/views/dashboards/manage/index.tsx index 53ff00990f0b39..93d341858294e9 100644 --- a/static/app/views/dashboards/manage/index.tsx +++ b/static/app/views/dashboards/manage/index.tsx @@ -208,7 +208,9 @@ function ManageDashboards() { pin: 'favorites', per_page: dashboardsLayout === GRID ? rowCount * columnCount : DASHBOARD_TABLE_NUM_ROWS, - ...(isOnlyPrebuilt ? {filter: DashboardFilter.ONLY_PREBUILT} : {}), + ...(isOnlyPrebuilt + ? {filter: DashboardFilter.ONLY_PREBUILT} + : {filter: DashboardFilter.EXCLUDE_PREBUILT}), }, }, ], From 876b9a552d8d2019f3fecee9e05150b08763df2b Mon Sep 17 00:00:00 2001 From: Lyn Nagara <1779792+lynnagara@users.noreply.github.com> Date: Fri, 27 Mar 2026 08:15:41 -0700 Subject: [PATCH 09/13] ref(cells): RpcOrganizationSlugReservation uses cell_name on the wire (#111597) flips the wire format from "region_name" to "cell_name". Alias is still present for deploy safety. --- .../control_organization_provisioning/model.py | 12 ++++++------ .../control_organization_provisioning/serial.py | 2 +- .../services/test_cell_organization_provisioning.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sentry/hybridcloud/services/control_organization_provisioning/model.py b/src/sentry/hybridcloud/services/control_organization_provisioning/model.py index e72dfd93ef27ce..90806036796a35 100644 --- a/src/sentry/hybridcloud/services/control_organization_provisioning/model.py +++ b/src/sentry/hybridcloud/services/control_organization_provisioning/model.py @@ -10,16 +10,16 @@ class RpcOrganizationSlugReservation(RpcModel): organization_id: int user_id: int | None slug: str - region_name: str + cell_name: str reservation_type: int @root_validator(pre=True) @classmethod - def _accept_cell_name(cls, values: dict[str, Any]) -> dict[str, Any]: - if "cell_name" in values and "region_name" not in values: - values["region_name"] = values.pop("cell_name") + def _accept_region_name(cls, values: dict[str, Any]) -> dict[str, Any]: + if "region_name" in values and "cell_name" not in values: + values["cell_name"] = values.pop("region_name") return values @property - def cell_name(self) -> str: - return self.region_name + def region_name(self) -> str: + return self.cell_name diff --git a/src/sentry/hybridcloud/services/control_organization_provisioning/serial.py b/src/sentry/hybridcloud/services/control_organization_provisioning/serial.py index 515773d81a3240..626aa6dfc3ed69 100644 --- a/src/sentry/hybridcloud/services/control_organization_provisioning/serial.py +++ b/src/sentry/hybridcloud/services/control_organization_provisioning/serial.py @@ -11,7 +11,7 @@ def serialize_slug_reservation( id=slug_reservation.id, organization_id=slug_reservation.organization_id, slug=slug_reservation.slug, - region_name=slug_reservation.cell_name, + cell_name=slug_reservation.cell_name, user_id=slug_reservation.user_id, reservation_type=slug_reservation.reservation_type, ) diff --git a/tests/sentry/hybridcloud/services/test_cell_organization_provisioning.py b/tests/sentry/hybridcloud/services/test_cell_organization_provisioning.py index bbe89289ca5053..371e24ba2f8d5a 100644 --- a/tests/sentry/hybridcloud/services/test_cell_organization_provisioning.py +++ b/tests/sentry/hybridcloud/services/test_cell_organization_provisioning.py @@ -254,7 +254,7 @@ def create_rpc_organization_slug_reservation(self, slug: str) -> RpcOrganization slug=slug, organization_id=self.provisioned_org.id, user_id=self.provisioning_user.id, - region_name="us", + cell_name="us", reservation_type=OrganizationSlugReservationType.TEMPORARY_RENAME_ALIAS.value, ) From 7652d18bb01640f7781dd26452afd94a19ad05ad Mon Sep 17 00:00:00 2001 From: Nikki Kapadia <72356613+nikkikapadia@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:27:44 -0400 Subject: [PATCH 10/13] chore(explore): create flag for errors on EAP (UI) (#111672) errors on EAP calls for new UI obv. This the feature flag for it --- src/sentry/features/temporary.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index b82748fba615b5..ebfd7086146fc2 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -116,6 +116,8 @@ def register_temporary_features(manager: FeatureManager) -> None: manager.add("organizations:dynamic-sampling-custom", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable dynamic sampling minimum sample rate manager.add("organizations:dynamic-sampling-minimum-sample-rate", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) + # Enable explore -> errors ui + manager.add("organizations:explore-errors", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable returning the migrated discover queries in explore saved queries manager.add("organizations:expose-migrated-discover-queries", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True) # Enable GenAI features such as Autofix and Issue Summary From 1636a61ccd82b3a52b77ab3426323a9c97b3a67c Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Fri, 27 Mar 2026 11:29:39 -0400 Subject: [PATCH 11/13] ref(issue-details): replace useRouter with specific hooks in useGroupDetailsRoute (#110122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of the React Router 6 cleanup — replaces `useRouter()` with specific hooks (`useNavigate`, `useLocation`, `useParams`, etc.) --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> --- .../issueDetails/useGroupDetailsRoute.tsx | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/static/app/views/issueDetails/useGroupDetailsRoute.tsx b/static/app/views/issueDetails/useGroupDetailsRoute.tsx index 5821725adced2e..63bb8322423103 100644 --- a/static/app/views/issueDetails/useGroupDetailsRoute.tsx +++ b/static/app/views/issueDetails/useGroupDetailsRoute.tsx @@ -1,18 +1,22 @@ -import type {RouteComponentProps} from 'sentry/types/legacyReactRouter'; +import type {PlainRoute} from 'sentry/types/legacyReactRouter'; import type {Organization} from 'sentry/types/organization'; import {normalizeUrl} from 'sentry/utils/url/normalizeUrl'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useParams} from 'sentry/utils/useParams'; -import {useRouter} from 'sentry/utils/useRouter'; +import {useRoutes} from 'sentry/utils/useRoutes'; import {Tab, TabPaths} from 'sentry/views/issueDetails/types'; -type RouteProps = RouteComponentProps<{groupId: string; eventId?: string}>; - -function getCurrentTab({router}: {router: RouteProps['router']}) { - const currentRoute = router.routes[router.routes.length - 1]; +function getCurrentTab({ + routes, + params, +}: { + params: Record; + routes: PlainRoute[]; +}) { + const currentRoute = routes[routes.length - 1]; // If we're in the tag details page ("/distributions/:tagKey/") - if (router.params.tagKey) { + if (params.tagKey) { return Tab.DISTRIBUTIONS; } return ( @@ -24,21 +28,23 @@ function getCurrentRouteInfo({ groupId, eventId, organization, - router, + routes, + params, }: { eventId: string | undefined; groupId: string; organization: Organization; - router: RouteProps['router']; + params: Record; + routes: PlainRoute[]; }): { baseUrl: string; currentTab: Tab; } { - const currentTab = getCurrentTab({router}); + const currentTab = getCurrentTab({routes, params}); const baseUrl = normalizeUrl( `/organizations/${organization.slug}/issues/${groupId}/${ - router.params.eventId && eventId ? `events/${eventId}/` : '' + params.eventId && eventId ? `events/${eventId}/` : '' }` ); @@ -50,12 +56,17 @@ export function useGroupDetailsRoute(): { currentTab: Tab; } { const organization = useOrganization(); - const params = useParams<{groupId: string; eventId?: string}>(); - const router = useRouter(); + const params = useParams<{ + groupId: string; + eventId?: string; + tagKey?: string; + }>(); + const routes = useRoutes(); return getCurrentRouteInfo({ groupId: params.groupId, eventId: params.eventId, organization, - router, + routes, + params, }); } From 77c7a04bb83b8f1e5072f313263ddf5436100f77 Mon Sep 17 00:00:00 2001 From: edwardgou-sentry <83961295+edwardgou-sentry@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:39:02 -0400 Subject: [PATCH 12/13] ref(dashboards): Use beta badge and tighten feature flag check for Dashboards Generate (#111712) - updates badge from experimental to beta - checks for areAiFeaturesAllowed --- static/app/views/dashboards/manage/index.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/static/app/views/dashboards/manage/index.tsx b/static/app/views/dashboards/manage/index.tsx index 93d341858294e9..4e90d37c651b40 100644 --- a/static/app/views/dashboards/manage/index.tsx +++ b/static/app/views/dashboards/manage/index.tsx @@ -170,6 +170,9 @@ function ManageDashboards() { const isOnlyPrebuilt = hasPrebuiltDashboards && urlFilter === DashboardFilter.ONLY_PREBUILT; + const areAiFeaturesAllowed = + !organization.hideAiFeatures && organization.features.includes('gen-ai-features'); + const [showTemplates, setShowTemplatesLocal] = useLocalStorageState( SHOW_TEMPLATES_KEY, shouldShowTemplates() @@ -657,9 +660,9 @@ function ManageDashboards() { )} - + {({hasFeature: hasAiGenerate}) => - hasAiGenerate ? ( + hasAiGenerate && areAiFeaturesAllowed ? ( {({ hasReachedDashboardLimit, @@ -682,7 +685,7 @@ function ManageDashboards() { label: ( {t('Generate dashboard')} - + ), onAction: () => onGenerateDashboard(), From 7c4cd2eb6fd577729668b40bea36c062475b5df9 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Fri, 27 Mar 2026 11:39:48 -0400 Subject: [PATCH 13/13] ref(profiling): replace useRouter with specific hooks in slowestFunctionsWidget (#110126) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of the React Router 6 cleanup — replaces `useRouter()` with specific hooks (`useNavigate`, `useLocation`, `useParams`, etc.) --- .../profiling/landing/slowestFunctionsWidget.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/static/app/views/profiling/landing/slowestFunctionsWidget.tsx b/static/app/views/profiling/landing/slowestFunctionsWidget.tsx index 2c071319f63118..561fa6c3b9cdcb 100644 --- a/static/app/views/profiling/landing/slowestFunctionsWidget.tsx +++ b/static/app/views/profiling/landing/slowestFunctionsWidget.tsx @@ -47,7 +47,6 @@ import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useProjects} from 'sentry/utils/useProjects'; -import {useRouter} from 'sentry/utils/useRouter'; import type {DataState} from 'sentry/views/profiling/useLandingAnalytics'; import {getProfileTargetId} from 'sentry/views/profiling/utils'; @@ -88,7 +87,6 @@ export function SlowestFunctionsWidget({ onDataState, }: SlowestFunctionsWidgetProps) { const navigate = useNavigate(); - const router = useRouter(); const location = useLocation(); const organization = useOrganization(); @@ -219,11 +217,14 @@ export function SlowestFunctionsWidget({ onChange={option => { setSortFunction(option.value as SortOption); setExpandedIndex(0); - const newQuery = omit(router.location.query, [cursorName]); - router.replace({ - pathname: router.location.pathname, - query: newQuery, - }); + const newQuery = omit(location.query, [cursorName]); + navigate( + { + pathname: location.pathname, + query: newQuery, + }, + {replace: true} + ); }} trigger={triggerProps => (