Skip to content

98080 #3

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+86 −1
Open

98080 #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/sentry/sentry_apps/services/app/impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
RpcSentryAppService,
SentryAppInstallationFilterArgs,
)
from sentry.sentry_apps.services.app.model import SentryAppUpdateArgs
from sentry.sentry_apps.services.app.serial import (
serialize_sentry_app,
serialize_sentry_app_component,
Expand Down Expand Up @@ -333,6 +334,33 @@ def get_published_sentry_apps_for_organization(
)
return [serialize_sentry_app(app) for app in published_apps]

def get_sentry_apps_for_organization(self, *, organization_id: int) -> list[RpcSentryApp]:
Copy link
Owner Author

Choose a reason for hiding this comment

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

/propel explain this function

Copy link
Owner Author

Choose a reason for hiding this comment

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

/propel explain this function

Copy link
Owner Author

Choose a reason for hiding this comment

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

/propel explain this function

Copy link

Choose a reason for hiding this comment

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

The get_sentry_apps_for_organization function, added at line 337 in src/sentry/sentry_apps/services/app/impl.py, is responsible for retrieving all active Sentry Apps for a given organization.

Here's a breakdown of its functionality:

  • Purpose: To fetch a list of Sentry Apps that are owned by a specific organization and are currently active (not in the process of being deleted).
  • Parameters: It takes one keyword argument:
    • organization_id (int): The unique identifier of the organization whose Sentry Apps are to be retrieved.
  • Logic:
    1. It queries the SentryApp database table.
    2. It filters these apps based on two conditions:
      • The owner_id of the Sentry App must match the provided organization_id.
      • The Sentry App must have an associated application (i.e., application__isnull=False).
    3. It then excludes any Sentry Apps that have a status of DELETION_IN_PROGRESS, ensuring only truly active apps are returned.
    4. Finally, it serializes the resulting SentryApp objects into RpcSentryApp format and returns them as a list.
  • Return Value: A list of RpcSentryApp objects, representing the active Sentry Apps for the specified organization.

Copy link
Owner Author

Choose a reason for hiding this comment

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

/propel explain this function

Copy link
Owner Author

Choose a reason for hiding this comment

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

/propel what this function does?

Copy link

Choose a reason for hiding this comment

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

The get_sentry_apps_for_organization function, located at line 337 in src/sentry/sentry_apps/services/app/impl.py, is designed to retrieve all active Sentry Apps associated with a specific organization.

Functionality Breakdown:

  • Purpose: To fetch a list of Sentry Apps that are owned by the given organization_id and are currently in an active state (i.e., not marked for deletion).
  • Parameters: It accepts one keyword argument:
    • organization_id (int): The unique identifier of the organization for which to retrieve Sentry Apps.
  • Logic:
    1. It queries the SentryApp database model.
    2. It filters these applications to include only those where the owner_id matches the provided organization_id and where an application object is associated (meaning application__isnull=False).
    3. It explicitly excludes any Sentry Apps that have a status of SentryAppStatus.DELETION_IN_PROGRESS.
    4. Finally, it serializes the filtered SentryApp objects into RpcSentryApp format and returns them as a list.
  • Return Value: A list of RpcSentryApp objects, each representing an active Sentry App belonging to the specified organization.

To improve clarity and completeness, I suggest expanding the docstring for this function to include details about its arguments and return value.

Suggested change
def get_sentry_apps_for_organization(self, *, organization_id: int) -> list[RpcSentryApp]:
"""
Get active Sentry Apps for a given organization.
Args:
organization_id (int): The ID of the organization.
Returns:
list[RpcSentryApp]: A list of serialized active Sentry Apps for the organization.
"""

⚡ Committable suggestion

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

"""
Get active Sentry Apps for a given organization
"""
sentry_apps = SentryApp.objects.filter(
owner_id=organization_id, application__isnull=False
).exclude(status=SentryAppStatus.DELETION_IN_PROGRESS)
return [serialize_sentry_app(app) for app in sentry_apps]

def update_sentry_app(
self,
*,
id: int,
attrs: SentryAppUpdateArgs,
) -> RpcSentryApp | None:
try:
sentry_app = SentryApp.objects.get(id=id)
except SentryApp.DoesNotExist:
return None

if len(attrs):
for k, v in attrs.items():
setattr(sentry_app, k, v)
sentry_app.save()

return serialize_sentry_app(sentry_app)

def get_internal_integrations(
self, *, organization_id: int, integration_name: str
) -> list[RpcSentryApp]:
Expand Down
6 changes: 6 additions & 0 deletions src/sentry/sentry_apps/services/app/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,9 @@ class SentryAppInstallationFilterArgs(TypedDict, total=False):
status: int
api_token_id: int
api_installation_token_id: str


class SentryAppUpdateArgs(TypedDict, total=False):
events: list[str]
name: str
# TODO add whatever else as needed
12 changes: 11 additions & 1 deletion src/sentry/sentry_apps/services/app/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
RpcSentryAppService,
SentryAppInstallationFilterArgs,
)
from sentry.sentry_apps.services.app.model import RpcSentryAppComponentContext
from sentry.sentry_apps.services.app.model import RpcSentryAppComponentContext, SentryAppUpdateArgs
from sentry.silo.base import SiloMode
from sentry.users.services.user import RpcUser

Expand Down Expand Up @@ -192,6 +192,16 @@ def get_published_sentry_apps_for_organization(
) -> list[RpcSentryApp]:
pass

@rpc_method
@abc.abstractmethod
def get_sentry_apps_for_organization(self, *, organization_id: int) -> list[RpcSentryApp]:
pass

@rpc_method
@abc.abstractmethod
def update_sentry_app(self, *, id: int, attrs: SentryAppUpdateArgs) -> RpcSentryApp | None:
pass

@rpc_method
@abc.abstractmethod
def get_internal_integrations(
Expand Down
41 changes: 41 additions & 0 deletions tests/sentry/sentry_apps/services/test_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,47 @@ def test_get_installation_org_id_by_token_id() -> None:
assert result is None


@django_db_all(transaction=True)
@all_silo_test
def test_get_sentry_apps_for_organization() -> None:
org = Factories.create_organization()
other_org = Factories.create_organization()

# Create internal integrations
Factories.create_internal_integration(
name="Test Integration",
organization_id=org.id,
)
Factories.create_internal_integration(
name="Test Integration",
organization_id=other_org.id,
)
result = app_service.get_sentry_apps_for_organization(organization_id=org.id)
assert len(result) == 1
assert result[0].owner_id == org.id


@django_db_all(transaction=True)
@all_silo_test
def test_update_sentry_app() -> None:
org = Factories.create_organization()

sentry_app = Factories.create_internal_integration(
name="Test Integration",
organization_id=org.id,
events=[
"issue.resolved",
"issue.unresolved",
"issue.ignored",
"issue.assigned",
"error.created",
],
)
result = app_service.update_sentry_app(id=sentry_app.id, attrs=dict(events=["error.created"]))
assert result
assert result.events == ["error.created"]


@django_db_all(transaction=True)
@all_silo_test
def test_get_internal_integrations() -> None:
Expand Down