Skip to content

Commit

Permalink
ref: fix types once HybridCloudForeignKey is properly nullable
Browse files Browse the repository at this point in the history
  • Loading branch information
asottile-sentry committed Jun 24, 2024
1 parent fc077e6 commit 91862d1
Show file tree
Hide file tree
Showing 13 changed files with 41 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ def get_attrs(
for item in item_list:
user_dct = users_by_id.get(str(item.user_id), None)
user_id = user_dct["id"] if user_dct else ""
inviter = inviters_by_id.get(item.inviter_id, None)
if item.inviter_id is not None:
inviter = inviters_by_id.get(item.inviter_id, None)
else:
inviter = None
external_users = external_users_map.get(user_id, [])
attrs[item] = {
"user": user_dct,
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/auth/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def accessible_project_ids(self) -> frozenset[int]:
def has_role_in_organization(
self, role: str, organization: Organization, user_id: int | None
) -> bool:
if self._member:
if self._member and self._member.user_id is not None:
return has_role_in_organization(
role=role, organization=organization, user_id=self._member.user_id
)
Expand Down Expand Up @@ -1129,7 +1129,7 @@ def from_member(
else:
scope_intersection = member.get_scopes()

if is_superuser or is_staff:
if (is_superuser or is_staff) and member.user_id is not None:
# "permissions" is a bit of a misnomer -- these are all admin level permissions, and the intent is that if you
# have them, you can only use them when you are acting, as a superuser or staff. This is intentional.
permissions = access_service.get_permissions_for_user(member.user_id)
Expand Down
9 changes: 6 additions & 3 deletions src/sentry/data_export/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ def email_success(self) -> None:
from sentry.utils.email import MessageBuilder

user_email = None
user = user_service.get_user(user_id=self.user_id)
if user:
user_email = user.email
if self.user_id is not None:
user = user_service.get_user(user_id=self.user_id)
if user:
user_email = user.email

# The following condition should never be true, but it's a safeguard in case someone manually calls this method
if self.date_finished is None or self.date_expired is None or self._get_file() is None:
Expand All @@ -115,6 +116,8 @@ def email_success(self) -> None:
def email_failure(self, message: str) -> None:
from sentry.utils.email import MessageBuilder

if self.user_id is None:
return
user = user_service.get_user(user_id=self.user_id)
if user is None:
return
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/pagerduty/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class PagerDutyClient(ApiClient):
integration_name = "pagerduty"
base_url = "https://events.pagerduty.com/v2/enqueue"

def __init__(self, integration_key: str, integration_id: int) -> None:
def __init__(self, integration_key: str, integration_id: int | None) -> None:
self.integration_key = integration_key
super().__init__(integration_id=integration_id)

Expand Down
5 changes: 4 additions & 1 deletion src/sentry/integrations/pagerduty/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ def send_incident_alert_notification(
org_integration_id = org_integration.id
else:
org_integrations = None
org_integration_id = infer_org_integration(integration_id=integration_id, ctx_logger=logger)
if integration_id is not None:
org_integration_id = infer_org_integration(
integration_id=integration_id, ctx_logger=logger
)
if org_integration_id:
org_integrations = integration_service.get_organization_integrations(
org_integration_ids=[org_integration_id]
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/models/grouphistory.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def get_prev_history_date(group, status):
project=group.project,
release=release,
team_id=team_id,
user_id=user_id, # type:ignore[misc]
user_id=user_id,
status=status,
prev_history=get_prev_history(group, status),
prev_history_date=get_prev_history_date(group, status),
Expand Down
8 changes: 6 additions & 2 deletions src/sentry/models/servicehook.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
from sentry.db.models.manager.base import BaseManager
from sentry.services.hybrid_cloud.app import app_service
from sentry.services.hybrid_cloud.app.model import RpcSentryApp

SERVICE_HOOK_EVENTS = [
"event.alert",
Expand Down Expand Up @@ -87,8 +88,11 @@ def created_by_sentry_app(self):
return self.application_id and bool(self.sentry_app)

@cached_property
def sentry_app(self):
return app_service.get_by_application_id(application_id=self.application_id)
def sentry_app(self) -> RpcSentryApp | None:
if self.application_id is None:
return None
else:
return app_service.get_by_application_id(application_id=self.application_id)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ def get_recipient_context(
)
else:
inviter_name = ""
inviter = user_service.get_user(user_id=self.pending_member.inviter_id)
if self.pending_member.inviter_id is not None:
inviter = user_service.get_user(user_id=self.pending_member.inviter_id)
else:
inviter = None
if inviter:
context["inviter_name"] = inviter.get_salutation_name()
context["inviter_name"] = inviter_name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ def set_member_in_cache(self, member: OrganizationMember) -> None:
"""
A way to set a member in a cache to avoid a query.
"""
self.member_by_user_id[member.user_id] = member
if member.user_id is not None:
self.member_by_user_id[member.user_id] = member

def determine_recipients(
self,
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/services/hybrid_cloud/access/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def query_sso_state(
def get_user_auth_state(
self,
*,
user_id: int,
user_id: int | None,
is_superuser: bool,
is_staff: bool,
organization_id: int | None,
Expand All @@ -111,7 +111,7 @@ def get_user_auth_state(

# Unfortunately we are unable to combine the is_superuser and is_staff
# into a single argument b/c query_sso_state specifically needs is_superuser
if is_superuser or is_staff:
if (is_superuser or is_staff) and user_id is not None:
# "permissions" is a bit of a misnomer -- these are all admin level permissions, and the intent is that if you
# have them, you can only use them when you are acting, as a superuser or staff. This is intentional.
permissions = list(self.get_permissions_for_user(user_id))
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/services/hybrid_cloud/replica/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def upsert_replicated_api_token(self, *, api_token: RpcApiToken, region_name: st
return

destination = ApiTokenReplica(
application_id=api_token.application_id, # type: ignore[misc]
application_id=api_token.application_id,
organization=organization,
application_is_active=api_token.application_is_active,
token=api_token.token,
Expand All @@ -183,7 +183,7 @@ def upsert_replicated_org_auth_token(self, *, token: RpcOrgAuthToken, region_nam
token_hashed=token.token_hashed,
name=token.name,
scope_list=token.scope_list,
created_by_id=token.created_by_id, # type: ignore[misc]
created_by_id=token.created_by_id,
date_deactivated=token.date_deactivated,
)
handle_replication(OrgAuthToken, destination)
Expand Down Expand Up @@ -300,7 +300,7 @@ def upsert_external_actor_replica(self, *, external_actor: RpcExternalActor) ->
organization_id=external_actor.organization_id,
user_id=external_actor.user_id,
provider=external_actor.provider,
team_id=external_actor.team_id, # type: ignore[misc]
team_id=external_actor.team_id,
integration_id=integration.id,
)
handle_replication(ExternalActor, destination, "externalactor_id")
Expand Down
2 changes: 2 additions & 0 deletions tests/sentry/incidents/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def test_simple(self):
self.incident, IncidentActivityType.COMMENT, user=self.user, comment="hello"
)
recipient = self.create_user()
assert activity.user_id is not None
user = user_service.get_user(user_id=activity.user_id)
assert user is not None
self.run_test(
Expand All @@ -145,6 +146,7 @@ def test_simple(self):
activity.type = IncidentActivityType.STATUS_CHANGE
activity.value = str(IncidentStatus.CLOSED.value)
activity.previous_value = str(IncidentStatus.WARNING.value)
assert activity.user_id is not None
user = user_service.get_user(user_id=activity.user_id)
assert user is not None
self.run_test(
Expand Down
8 changes: 4 additions & 4 deletions tests/sentry/mediators/project_rules/test_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ def test_update_owner(self):
self.updater.call()
with assume_test_silo_mode_of(User):
self.user = User.objects.get(id=self.user.id)
assert self.rule.owner_user_id == self.user.id
assert self.rule.owner_team_id is None

assert (self.rule.owner_user_id, self.rule.owner_team_id) == (self.user.id, None)

team = self.create_team()
self.updater.owner = Actor.from_id(team_id=team.id)
self.updater.call()
assert self.rule.owner_team_id == team.id
assert self.rule.owner_user_id is None

assert (self.rule.owner_user_id, self.rule.owner_team_id) == (None, team.id)

self.updater.owner = None
self.updater.call()
Expand Down

0 comments on commit 91862d1

Please sign in to comment.