Skip to content

fix(hc): Introduce Organization.get_bulk_owner_profiles #68756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 22, 2024
44 changes: 43 additions & 1 deletion src/sentry/models/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from sentry.roles.manager import Role
from sentry.services.hybrid_cloud.notifications import notifications_service
from sentry.services.hybrid_cloud.organization_mapping import organization_mapping_service
from sentry.services.hybrid_cloud.user import RpcUser
from sentry.services.hybrid_cloud.user import RpcUser, RpcUserContact
from sentry.services.hybrid_cloud.user.service import user_service
from sentry.types.organization import OrganizationAbsoluteUrlMixin
from sentry.utils.http import is_using_customer_domain
Expand Down Expand Up @@ -329,6 +329,48 @@ def default_owner_id(self):
self._default_owner_id = owners[0].id
return self._default_owner_id

@classmethod
def _get_bulk_owner_ids(cls, organizations: Collection[Organization]) -> dict[int, int]:
from sentry.models.organizationmember import OrganizationMember

owner_id_table: dict[int, int] = {}
org_ids_to_query: list[int] = []
for org in organizations:
default_owner = getattr(org, "_default_owner", None)
if default_owner:
owner_id_table[org.id] = default_owner.id
else:
org_ids_to_query.append(org.id)

if org_ids_to_query:
queried_owner_ids = OrganizationMember.objects.filter(
organization_id__in=org_ids_to_query, role=roles.get_top_dog().id
).values_list("organization_id", "user_id")

for (org_id, user_id) in queried_owner_ids:
# An org may have multiple owners. Here we mimic the behavior of
# `get_default_owner`, which is to use the first one in the query
# result's iteration order.
if org_id not in owner_id_table:
owner_id_table[org_id] = user_id

return owner_id_table

@classmethod
def get_bulk_owner_contacts(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a test for this method as it is public interface that getsentry will rely on.

cls, organizations: Collection[Organization]
) -> dict[int, RpcUserContact]:
owner_id_table = cls._get_bulk_owner_ids(organizations)

contacts = user_service.get_bulk_contact_info(user_ids=list(owner_id_table.values()))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This safe to roll out at the same time as the new service method, because nothing is going to hit get_bulk_owner_contacts until https://github.com/getsentry/getsentry/pull/13623 is merged.

contact_table = {c.id: c for c in contacts}

return {
org_id: contact_table[user_id]
for (org_id, user_id) in owner_id_table.items()
if user_id in contact_table
}

def has_single_owner(self):
owners = list(
self.get_members_with_org_roles([roles.get_top_dog().id])[:2].values_list(
Expand Down
10 changes: 9 additions & 1 deletion src/sentry/services/hybrid_cloud/user/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
UserSerializeType,
UserUpdateArgs,
)
from sentry.services.hybrid_cloud.user.model import RpcVerifyUserEmail, UserIdEmailArgs
from sentry.services.hybrid_cloud.user.model import (
RpcUserContact,
RpcVerifyUserEmail,
UserIdEmailArgs,
)
from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user, serialize_user_avatar
from sentry.services.hybrid_cloud.user.service import UserService
from sentry.signals import user_signup
Expand Down Expand Up @@ -283,6 +287,10 @@ def get_user_avatar(self, *, user_id: int) -> RpcAvatar | None:

return serialize_user_avatar(avatar=possible_avatar)

def get_bulk_contact_info(self, *, user_ids: list[int]) -> list[RpcUserContact]:
users = User.objects.filter(id__in=user_ids).values_list("id", "name", "email")
return [RpcUserContact(id=id, name=name, email=email) for (id, name, email) in users]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we have other flows in the application that could benefit from loading less data when needing to show lists of users 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted to generalize this in #68847.


class _UserFilterQuery(
FilterQueryDatabaseImpl[User, UserFilterArgs, RpcUser, UserSerializeType],
):
Expand Down
6 changes: 6 additions & 0 deletions src/sentry/services/hybrid_cloud/user/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ class RpcUserEmail(RpcModel):
is_verified: bool = False


class RpcUserContact(RpcModel):
id: int = -1
name: str = ""
email: str = ""


class RpcAuthenticator(RpcModel):
id: int = 0
user_id: int = -1
Expand Down
12 changes: 11 additions & 1 deletion src/sentry/services/hybrid_cloud/user/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
UserSerializeType,
UserUpdateArgs,
)
from sentry.services.hybrid_cloud.user.model import RpcAvatar, RpcVerifyUserEmail, UserIdEmailArgs
from sentry.services.hybrid_cloud.user.model import (
RpcAvatar,
RpcUserContact,
RpcVerifyUserEmail,
UserIdEmailArgs,
)
from sentry.silo import SiloMode


Expand Down Expand Up @@ -195,6 +200,11 @@ def verify_user_emails(
def get_user_avatar(self, *, user_id: int) -> RpcAvatar | None:
pass

@rpc_method
@abstractmethod
def get_bulk_contact_info(self, *, user_ids: list[int]) -> list[RpcUserContact]:
pass


@back_with_silo_cache("user_service.get_user", SiloMode.REGION, RpcUser)
def get_user(user_id: int) -> RpcUser | None:
Expand Down
Loading