ref(hc): Remove get_orgs() #54635
ref(hc): Remove get_orgs() #54635
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #54635 +/- ##
==========================================
- Coverage 79.75% 79.75% -0.01%
==========================================
Files 4995 5001 +6
Lines 212163 212385 +222
Branches 36163 36196 +33
==========================================
+ Hits 169206 169380 +174
- Misses 37750 37793 +43
- Partials 5207 5212 +5
|
| @@ -77,17 +80,17 @@ class SentryAppsPermission(SentryPermission): | |||
| "POST": ("org:write", "org:admin"), | |||
| } | |||
|
|
|||
| def has_object_permission(self, request: Request, view, organization): | |||
There was a problem hiding this comment.
SentryApps will be control silo, which means they won't have direct access to organizations. the context here represents an RPC lookup of org details.
| @@ -89,28 +93,11 @@ def put(self, request: Request, user, notification_type) -> Response: | |||
|
|
|||
| # Make sure that the IDs we are going to update are a subset of the | |||
| # user's list of organizations or projects. | |||
| parents = ( | |||
There was a problem hiding this comment.
Requiring validation of project is and organization ids forces a fan out across all regions for a user. In reality, these values are not foreign keys. We also frequently "leave behind" notification settings without clearing them in all cases, so letting arbitrary values from a user isn't particularly problematic. Having notification settings for a project or org you don't belong to doesn't grant any access or necessarily grant any notifications.
| from sentry.services.hybrid_cloud.user.service import user_service | ||
|
|
||
|
|
||
| @region_silo_endpoint |
There was a problem hiding this comment.
Swapping this to a region endpoint. In the future, we plan to have the frontend query a user's organization settings per non single tenant region they belong to. Pushing this to the frontend makes sense in the long term.
| def requires_feature( | ||
| feature: str, any_org: Optional[bool] = None | ||
| ) -> Callable[[EndpointFunc], EndpointFunc]: | ||
| def requires_feature(feature: str) -> Callable[[EndpointFunc], EndpointFunc]: |
There was a problem hiding this comment.
@corps We can just delete this function as it's no longer used anywhere.
|
PR reverted: 4235ed0 |
This reverts commit de831d0. Co-authored-by: corps <593850+corps@users.noreply.github.com>
user.get_orgs() is problematic, because the Organization objects associated to a user via OrganizationMember can only ever be per region, and not contain all organizations a user belongs to globally.
This PR attempts to use separate means of getting the same question answered but with more specific implementations, so we can extract the
user.get_orgs()which is ambiguous.For instance,
Organization.get_for_user_idscan only mean within a region scope. butuser_service.get_organizationswill include global organizations.