diff --git a/CHANGELOG.md b/CHANGELOG.md index 59d210c099..525600c624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `b64decode` Jinja2 template helper filter by @jorgeav ([#3242](https://github.com/grafana/oncall/pull/3242)) +## v1.3.53 (2023-11-03) + ### Fixed - Fix db migration for mobile app @Ferril ([#3260](https://github.com/grafana/oncall/pull/3260)) diff --git a/docker-compose.yml b/docker-compose.yml index 2dadfc3a03..b7476986ce 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,7 @@ x-environment: &oncall-environment PROMETHEUS_EXPORTER_SECRET: ${PROMETHEUS_EXPORTER_SECRET:-} REDIS_URI: redis://redis:6379/0 DJANGO_SETTINGS_MODULE: settings.hobby - CELERY_WORKER_QUEUE: "default,critical,long,slack,telegram,webhook,retry,celery" + CELERY_WORKER_QUEUE: "default,critical,long,slack,telegram,webhook,retry,celery,grafana" CELERY_WORKER_CONCURRENCY: "1" CELERY_WORKER_MAX_TASKS_PER_CHILD: "100" CELERY_WORKER_SHUTDOWN_INTERVAL: "65m" diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index 42a996f7c3..d82409405c 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -197,28 +197,3 @@ def unpage_user(alert_group: AlertGroup, user: User, from_user: User) -> None: def user_is_oncall(user: User) -> bool: schedules_with_oncall_users = get_oncall_users_for_multiple_schedules(OnCallSchedule.objects.related_to_user(user)) return user.pk in {user.pk for _, users in schedules_with_oncall_users.items() for user in users} - - -def integration_is_notifiable(integration: AlertReceiveChannel) -> bool: - """ - Returns true if: - - the integration has more than one channel filter associated with it - - the default channel filter has at least one notification method specified or an escalation chain associated with it - """ - if integration.channel_filters.count() > 1: - return True - - default_channel_filter = integration.default_channel_filter - if not default_channel_filter: - return False - - organization = integration.organization - notify_via_slack = organization.slack_is_configured and default_channel_filter.notify_in_slack - notify_via_telegram = organization.telegram_is_configured and default_channel_filter.notify_in_telegram - - notify_via_chatops = notify_via_slack or notify_via_telegram - custom_messaging_backend_configured = default_channel_filter.notification_backends is not None - - return ( - default_channel_filter.escalation_chain is not None or notify_via_chatops or custom_messaging_backend_configured - ) diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index 96d5b74610..2c0ae9f820 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -8,7 +8,6 @@ DirectPagingUserTeamValidationError, _construct_title, direct_paging, - integration_is_notifiable, unpage_user, user_is_oncall, ) @@ -292,67 +291,3 @@ def _title(middle_portion: str) -> str: assert _construct_title(from_user, team, multiple_users) == _title( f"{team.name}, {user1.username}, {user2.username} and {user3.username}" ) - - -@pytest.mark.django_db -def test_integration_is_notifiable( - make_organization, - make_alert_receive_channel, - make_channel_filter, - make_escalation_chain, - make_slack_team_identity, - make_telegram_channel, -): - organization = make_organization() - - # integration has no default channel filter - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=False) - assert integration_is_notifiable(arc) is False - - # integration has more than one channel filter - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=False) - make_channel_filter(arc, is_default=False) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is setup to notify via slack but Slack is not configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=True) - assert integration_is_notifiable(arc) is False - - # integration's default channel filter is setup to notify via slack and Slack is configured for the org - arc = make_alert_receive_channel(organization) - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() - - make_channel_filter(arc, is_default=True, notify_in_slack=True) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is setup to notify via telegram but Telegram is not configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) - assert integration_is_notifiable(arc) is False - - # integration's default channel filter is setup to notify via telegram and Telegram is configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) - make_telegram_channel(organization) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is contactable via a custom messaging backend - arc = make_alert_receive_channel(organization) - make_channel_filter( - arc, - is_default=True, - notify_in_slack=False, - notification_backends={"MSTEAMS": {"channel": "test", "enabled": True}}, - ) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter has an escalation chain attached to it - arc = make_alert_receive_channel(organization) - escalation_chain = make_escalation_chain(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, escalation_chain=escalation_chain) - assert integration_is_notifiable(arc) is True diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index a0332dc7d1..2fd9fd915c 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -270,14 +270,19 @@ class Meta: ] -class UserLongSerializer(UserSerializer): +class UserIsCurrentlyOnCallSerializer(UserShortSerializer, EagerLoadingMixin): context: UserSerializerContext teams = FastTeamSerializer(read_only=True, many=True) is_currently_oncall = serializers.SerializerMethodField() - class Meta(UserSerializer.Meta): - fields = UserSerializer.Meta.fields + [ + SELECT_RELATED = ["organization"] + PREFETCH_RELATED = ["teams"] + + class Meta(UserShortSerializer.Meta): + fields = UserShortSerializer.Meta.fields + [ + "name", + "timezone", "teams", "is_currently_oncall", ] diff --git a/engine/apps/api/tests/test_team.py b/engine/apps/api/tests/test_team.py index 69a8afbe9d..f66b44360e 100644 --- a/engine/apps/api/tests/test_team.py +++ b/engine/apps/api/tests/test_team.py @@ -28,6 +28,25 @@ def get_payload_from_team(team, long=False): return payload +@pytest.mark.django_db +def test_get_team(make_organization_and_user_with_plugin_token, make_team, make_user_auth_headers): + organization, user, token = make_organization_and_user_with_plugin_token() + team = make_team(organization) + + client = APIClient() + + # team exists + url = reverse("api-internal:team-detail", kwargs={"pk": team.public_primary_key}) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert response.json() == get_payload_from_team(team) + + # 404 scenario + url = reverse("api-internal:team-detail", kwargs={"pk": "asdfasdflkjlkajsdf"}) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db def test_list_teams( make_organization, @@ -100,7 +119,20 @@ def test_list_teams_only_include_notifiable_teams( client = APIClient() url = reverse("api-internal:team-list") - with patch("apps.api.views.team.integration_is_notifiable", side_effect=lambda obj: obj.id == arc1.id): + def mock_get_notifiable_direct_paging_integrations(): + class MockRelatedManager: + def filter(self, *args, **kwargs): + return self + + def values_list(self, *args, **kwargs): + return [arc1.team.pk] + + return MockRelatedManager() + + with patch( + "apps.user_management.models.Organization.get_notifiable_direct_paging_integrations", + side_effect=mock_get_notifiable_direct_paging_integrations, + ): response = client.get( f"{url}?only_include_notifiable_teams=true&include_no_team=false", format="json", diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index e918762ce2..8c15db3491 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -1935,7 +1935,7 @@ def test_users_is_currently_oncall_attribute_works_properly( schedule.refresh_ical_final_schedule() client = APIClient() - url = f"{reverse('api-internal:user-list')}?short=false" + url = f"{reverse('api-internal:user-list')}?is_currently_oncall=all" response = client.get(url, format="json", **make_user_auth_headers(user1, token)) oncall_statuses = { diff --git a/engine/apps/api/views/team.py b/engine/apps/api/views/team.py index 47c10e118a..72c10770ad 100644 --- a/engine/apps/api/views/team.py +++ b/engine/apps/api/views/team.py @@ -4,7 +4,6 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response -from apps.alerts.paging import integration_is_notifiable from apps.api.permissions import RBACPermission from apps.api.serializers.team import TeamLongSerializer, TeamSerializer from apps.auth_token.auth import PluginAuthentication @@ -14,7 +13,13 @@ from common.api_helpers.mixins import PublicPrimaryKeyMixin -class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet): +class TeamViewSet( + PublicPrimaryKeyMixin, + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + viewsets.GenericViewSet, +): authentication_classes = ( MobileAppAuthTokenAuthentication, PluginAuthentication, @@ -61,14 +66,11 @@ def list(self, request, *args, **kwargs): queryset = self.filter_queryset(self.get_queryset()) if self.request.query_params.get("only_include_notifiable_teams", "false") == "true": - # filters down to only teams that have a direct paging integration that is "notifiable" - orgs_direct_paging_integrations = self.request.user.organization.get_direct_paging_integrations() - notifiable_direct_paging_integrations = [ - i for i in orgs_direct_paging_integrations if integration_is_notifiable(i) - ] - team_ids = [i.team.pk for i in notifiable_direct_paging_integrations if i.team is not None] - - queryset = queryset.filter(pk__in=team_ids) + queryset = queryset.filter( + pk__in=self.request.user.organization.get_notifiable_direct_paging_integrations() + .filter(team__isnull=False) + .values_list("team__pk", flat=True) + ) queryset = queryset.order_by("name") diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index c6a51fc71e..06076e440a 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -29,7 +29,7 @@ CurrentUserSerializer, FilterUserSerializer, UserHiddenFieldsSerializer, - UserLongSerializer, + UserIsCurrentlyOnCallSerializer, UserSerializer, ) from apps.api.throttlers import ( @@ -238,20 +238,14 @@ def _get_is_currently_oncall_query_param(self) -> str: return self.request.query_params.get("is_currently_oncall", "").lower() def _is_currently_oncall_request(self) -> bool: - return self._get_is_currently_oncall_query_param() in ["true", "false"] - - def _is_long_request(self) -> bool: - return self.request.query_params.get("short", "true").lower() == "false" - - def _is_currently_oncall_or_long_request(self) -> bool: - return self._is_currently_oncall_request() or self._is_long_request() + return self._get_is_currently_oncall_query_param() in ["true", "false", "all"] def get_serializer_context(self): context = super().get_serializer_context() context.update( { "schedules_with_oncall_users": self.schedules_with_oncall_users - if self._is_currently_oncall_or_long_request() + if self._is_currently_oncall_request() else {} } ) @@ -268,8 +262,8 @@ def get_serializer_class(self): if is_list_request and is_filters_request: return self.get_filter_serializer_class() - elif is_list_request and self._is_currently_oncall_or_long_request(): - return UserLongSerializer + elif is_list_request and self._is_currently_oncall_request(): + return UserIsCurrentlyOnCallSerializer is_users_own_data = kwargs.get("pk") is not None and kwargs.get("pk") == user.public_primary_key has_admin_permission = user_is_authorized(user, [RBACPermission.Permissions.USER_SETTINGS_ADMIN]) @@ -296,8 +290,7 @@ def list(self, request, *args, **kwargs) -> Response: def _get_oncall_user_ids(): return {user.pk for _, users in self.schedules_with_oncall_users.items() for user in users} - is_currently_oncall_query_param = self._get_is_currently_oncall_query_param() - if is_currently_oncall_query_param == "true": + if (is_currently_oncall_query_param := self._get_is_currently_oncall_query_param()) == "true": # client explicitly wants to filter out users that are on-call queryset = queryset.filter(pk__in=_get_oncall_user_ids()) elif is_currently_oncall_query_param == "false": diff --git a/engine/apps/heartbeat/tasks.py b/engine/apps/heartbeat/tasks.py index d02fac9b0f..a00987e259 100644 --- a/engine/apps/heartbeat/tasks.py +++ b/engine/apps/heartbeat/tasks.py @@ -45,8 +45,14 @@ def _get_timeout_expression() -> ExpressionWrapper: # * is enabled, # * is not already expired, # * last check in was before the timeout period start - expired_heartbeats = enabled_heartbeats.select_for_update().filter( - last_heartbeat_time__lte=F("period_start"), previous_alerted_state_was_life=True + expired_heartbeats = ( + enabled_heartbeats.select_for_update() + .filter( + last_heartbeat_time__lte=F("period_start"), + previous_alerted_state_was_life=True, + alert_receive_channel__organization__deleted_at__isnull=True, + ) + .select_related("alert_receive_channel") ) # Schedule alert creation for each expired heartbeat after transaction commit for heartbeat in expired_heartbeats: diff --git a/engine/apps/heartbeat/tests/test_integration_heartbeat.py b/engine/apps/heartbeat/tests/test_integration_heartbeat.py index e2cdb8c3f7..4b0c89a03b 100644 --- a/engine/apps/heartbeat/tests/test_integration_heartbeat.py +++ b/engine/apps/heartbeat/tests/test_integration_heartbeat.py @@ -25,10 +25,10 @@ def test_check_heartbeats( assert mock_create_alert_apply_async.call_count == 0 # Prepare heartbeat - team, _ = make_organization_and_user() + organization, _ = make_organization_and_user() timeout = 60 last_heartbeat_time = timezone.now() - alert_receive_channel = make_alert_receive_channel(team, integration=integration) + alert_receive_channel = make_alert_receive_channel(organization, integration=integration) integration_heartbeat = make_integration_heartbeat( alert_receive_channel, timeout, last_heartbeat_time=last_heartbeat_time, previous_alerted_state_was_life=True ) @@ -78,3 +78,14 @@ def test_check_heartbeats( result = check_heartbeats() assert result == "Found 0 expired and 0 restored heartbeats" assert mock_create_alert_apply_async.call_count == 0 + + # Hearbeat expires, but organization was deleted, don't send an alert + integration_heartbeat.refresh_from_db() + integration_heartbeat.last_heartbeat_time = timezone.now() - timezone.timedelta(seconds=timeout * 10) + integration_heartbeat.save() + organization.delete() + with patch.object(create_alert, "apply_async") as mock_create_alert_apply_async: + with django_capture_on_commit_callbacks(execute=True): + result = check_heartbeats() + assert result == "Found 0 expired and 0 restored heartbeats" + assert mock_create_alert_apply_async.call_count == 0 diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index dd40ac6256..9bd5449881 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -534,26 +534,54 @@ def _get_team_select_blocks( slack_user_identity: "SlackUserIdentity", organization: "Organization", is_selected: bool, - value: "Team", + value: typing.Optional["Team"], input_id_prefix: str, ) -> Block.AnyBlocks: + blocks: Block.AnyBlocks = [] user = slack_user_identity.get_user(organization) # TODO: handle None - teams = user.available_teams + teams = ( + user.organization.get_notifiable_direct_paging_integrations() + .filter(team__isnull=False) + .values_list("team__pk", "team__name") + ) + + direct_paging_info_msg = { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": ( + "*Note*: You can only page teams which have a Direct Paging integration that is configured. " + "" + ), + }, + ], + } + + if not teams: + direct_paging_info_msg["elements"][0][ + "text" + ] += ". There are currently no teams which have a Direct Paging integration that is configured." + blocks.append(direct_paging_info_msg) + return blocks team_options: typing.List[CompositionObjectOption] = [] initial_option_idx = 0 for idx, team in enumerate(teams): - if team == value: + team_pk, team_name = team + team_pk_str = str(team_pk) + + if value == team_pk_str: initial_option_idx = idx team_options.append( { "text": { "type": "plain_text", - "text": f"{team.name}", + "text": team_name, "emoji": True, }, - "value": f"{team.pk}", + "value": team_pk_str, } ) @@ -574,10 +602,11 @@ def _get_team_select_blocks( "optional": True, } - blocks: Block.AnyBlocks = [team_select] + blocks.append(team_select) # No context block if no team selected if not is_selected: + blocks.append(direct_paging_info_msg) return blocks team_select["element"]["initial_option"] = team_options[initial_option_idx] diff --git a/engine/apps/slack/tests/test_scenario_steps/test_paging.py b/engine/apps/slack/tests/test_scenario_steps/test_paging.py index 91e6cf5010..2821b8e616 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_paging.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_paging.py @@ -4,6 +4,7 @@ import pytest from django.utils import timezone +from apps.alerts.models import AlertReceiveChannel from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb from apps.slack.scenarios.paging import ( DIRECT_PAGING_MESSAGE_INPUT_ID, @@ -19,6 +20,7 @@ Policy, StartDirectPaging, _get_organization_select, + _get_team_select_blocks, ) from apps.user_management.models import Organization @@ -243,6 +245,20 @@ def test_trigger_paging_additional_responders(make_organization_and_user_with_sl mock_direct_paging.called_once_with(organization, user, "The Message", team, [(user, True)]) +@pytest.mark.django_db +def test_page_team(make_organization_and_user_with_slack_identities, make_team): + organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + payload = make_slack_payload(organization=organization, team=team) + + step = FinishDirectPaging(slack_team_identity) + with patch("apps.slack.scenarios.paging.direct_paging") as mock_direct_paging: + with patch.object(step._slack_client, "api_call"): + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + mock_direct_paging.called_once_with(organization, user, "The Message", team) + + @pytest.mark.django_db def test_get_organization_select(make_organization): organization = make_organization(org_title="Organization", stack_slug="stack_slug") @@ -251,3 +267,75 @@ def test_get_organization_select(make_organization): assert len(select["element"]["options"]) == 1 assert select["element"]["options"][0]["value"] == str(organization.pk) assert select["element"]["options"][0]["text"]["text"] == "Organization (stack_slug)" + + +@pytest.mark.django_db +def test_get_team_select_blocks( + make_organization_and_user_with_slack_identities, + make_team, + make_alert_receive_channel, + make_escalation_chain, + make_channel_filter, +): + info_msg = ( + "*Note*: You can only page teams which have a Direct Paging integration that is configured. " + "" + ) + + input_id_prefix = "nmxcnvmnxv" + + # no team selected - no team direct paging integrations available + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) + + assert len(blocks) == 1 + + context_block = blocks[0] + assert context_block["type"] == "context" + assert ( + context_block["elements"][0]["text"] + == info_msg + ". There are currently no teams which have a Direct Paging integration that is configured." + ) + + # no team selected - 1 team direct paging integration available + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + arc = make_alert_receive_channel(organization, team=team, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + escalation_chain = make_escalation_chain(organization) + make_channel_filter(arc, is_default=True, escalation_chain=escalation_chain) + + blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) + + assert len(blocks) == 2 + input_block, context_block = blocks + + team_option = {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} + + assert input_block["type"] == "input" + assert len(input_block["element"]["options"]) == 1 + assert input_block["element"]["options"] == [team_option] + assert context_block["elements"][0]["text"] == info_msg + + # team selected + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + arc = make_alert_receive_channel(organization, team=team, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + escalation_chain = make_escalation_chain(organization) + make_channel_filter(arc, is_default=True, escalation_chain=escalation_chain) + + blocks = _get_team_select_blocks(slack_user_identity, organization, True, team.pk, input_id_prefix) + + assert len(blocks) == 2 + input_block, context_block = blocks + + team_option = {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} + + assert input_block["type"] == "input" + assert len(input_block["element"]["options"]) == 1 + assert input_block["element"]["options"] == [team_option] + assert input_block["element"]["initial_option"] == team_option + + assert ( + context_block["elements"][0]["text"] + == f"Integration <{arc.web_link}|{arc.verbal_name}> will be used for notification." + ) diff --git a/engine/apps/telegram/tests/test_button_update_handler.py b/engine/apps/telegram/tests/test_button_update_handler.py index 5ae49f7d68..657c9e460a 100644 --- a/engine/apps/telegram/tests/test_button_update_handler.py +++ b/engine/apps/telegram/tests/test_button_update_handler.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest @@ -6,8 +6,13 @@ from apps.telegram.updates.update_handlers.button_press import ButtonPressHandler +@patch( + "apps.telegram.updates.update_handlers.button_press.ButtonPressHandler._get_alert_group_from_message", + return_value=None, +) @pytest.mark.django_db def test_get_action_context( + mocked_get_alert_group_from_message, make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, diff --git a/engine/apps/telegram/updates/update_handlers/button_press.py b/engine/apps/telegram/updates/update_handlers/button_press.py index d629462cbe..0c8c13f85f 100644 --- a/engine/apps/telegram/updates/update_handlers/button_press.py +++ b/engine/apps/telegram/updates/update_handlers/button_press.py @@ -5,7 +5,7 @@ from apps.alerts.constants import ActionSource from apps.alerts.models import AlertGroup from apps.api.permissions import RBACPermission, user_is_authorized -from apps.telegram.models import TelegramToUserConnector +from apps.telegram.models import TelegramMessage, TelegramToUserConnector from apps.telegram.renderers.keyboard import CODE_TO_ACTION_MAP, Action from apps.telegram.updates.update_handlers import UpdateHandler from apps.telegram.utils import CallbackQueryFactory @@ -45,6 +45,15 @@ def process_update(self) -> None: self.update.callback_query.answer(PERMISSION_DENIED, show_alert=True) logger.info(f"User {user} has no permission to trigger '{fn.__name__}'") + def _get_alert_group_from_message(self) -> Optional[AlertGroup]: + alert_group = None + if self.update.message: + telegram_message = TelegramMessage.objects.get( + message_id=self.update.message.message_id, chat_id=self.update.message.chat.id + ) + alert_group = telegram_message.alert_group + return alert_group + def _get_user(self, action_context: ActionContext) -> Optional[User]: connector = TelegramToUserConnector.objects.filter( telegram_chat_id=self.update.effective_user.id, @@ -60,12 +69,15 @@ def _check_permission(user: Optional[User], alert_group: AlertGroup) -> bool: has_permission = user_is_authorized(user, [RBACPermission.Permissions.CHATOPS_WRITE]) return user.organization == alert_group.channel.organization and has_permission - @classmethod - def _get_action_context(cls, data: str) -> ActionContext: + def _get_action_context(self, data: str) -> ActionContext: args = CallbackQueryFactory.decode_data(data) - alert_group_pk = args[0] - alert_group = AlertGroup.objects.get(pk=alert_group_pk) + # Try to get alert group from telegram message, because encoded data is not valid for migrated organizations + alert_group = self._get_alert_group_from_message() + + if alert_group is None: + alert_group_pk = args[0] + alert_group = AlertGroup.objects.get(pk=alert_group_pk) action_value = args[1] try: @@ -77,7 +89,7 @@ def _get_action_context(cls, data: str) -> ActionContext: action_name = action_value action = Action(action_name) - action_data = args[2] if len(args) >= 3 and not cls._is_oncall_identifier(args[2]) else None + action_data = args[2] if len(args) >= 3 and not self._is_oncall_identifier(args[2]) else None return ActionContext(alert_group=alert_group, action=action, action_data=action_data) diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index 5a74531ae2..70fcf8e002 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -6,6 +6,7 @@ from django.conf import settings from django.core.validators import MinLengthValidator from django.db import models +from django.db.models import Count, Q from django.utils import timezone from mirage import fields as mirage_fields @@ -298,10 +299,32 @@ def set_general_log_channel(self, channel_id, channel_name, user): new_channel=channel_name, ) - def get_direct_paging_integrations(self) -> "RelatedManager['AlertReceiveChannel']": + def get_notifiable_direct_paging_integrations(self) -> "RelatedManager['AlertReceiveChannel']": + """ + in layman's terms, this filters down an organization's integrations to ones which meet the following criterias: + - the integration is a direct paging integration + + AND at-least one of the following conditions are true for the integration: + - have more than one channel filter associated with it + - OR the organization has either Slack or Telegram configured (as the direct paging integration + would automatically be configured to be notified via these channel(s)) + - OR the default channel filter associated with the integration has an escalation chain associated with it + - OR the default channel filter associated with the integration is contactable via a custom + messaging backend + """ from apps.alerts.models import AlertReceiveChannel - return self.alert_receive_channels.filter(integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + return self.alert_receive_channels.annotate( + num_channel_filters=Count("channel_filters"), + # used to determine if the organization has telegram configured + num_org_telegram_channels=Count("organization__telegram_channel"), + ).filter( + Q(num_channel_filters__gt=1) + | (Q(organization__slack_team_identity__isnull=False) | Q(num_org_telegram_channels__gt=0)) + | Q(channel_filters__is_default=True, channel_filters__escalation_chain__isnull=False) + | Q(channel_filters__is_default=True, channel_filters__notification_backends__isnull=False), + integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, + ) @property def web_link(self): @@ -312,14 +335,6 @@ def web_link_with_uuid(self): # It's a workaround to pass some unique identifier to the oncall gateway while proxying telegram requests return urljoin(self.grafana_url, f"a/grafana-oncall-app/?oncall-uuid={self.uuid}") - @property - def slack_is_configured(self) -> bool: - return self.slack_team_identity is not None - - @property - def telegram_is_configured(self) -> bool: - return self.telegram_channel.count() > 0 - @classmethod def __str__(self): return f"{self.pk}: {self.org_title}" diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 692eae9080..91915500b5 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -198,47 +198,74 @@ def test_organization_hard_delete( @pytest.mark.django_db -def test_slack_is_configured(make_organization, make_slack_team_identity): - organization = make_organization() - - assert organization.slack_is_configured is False +def test_get_notifiable_direct_paging_integrations( + make_organization, + make_alert_receive_channel, + make_channel_filter, + make_escalation_chain, + make_slack_team_identity, + make_telegram_channel, +): + def _make_org_and_arc(**arc_kwargs): + org = make_organization() + arc = make_alert_receive_channel(org, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, **arc_kwargs) + return org, arc + + def _assert(org, arc, should_be_returned=True): + notifiable_direct_paging_integrations = org.get_notifiable_direct_paging_integrations() + if should_be_returned: + assert arc in notifiable_direct_paging_integrations + else: + assert arc not in notifiable_direct_paging_integrations + + # integration has no default channel filter + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=False) + _assert(org, arc, should_be_returned=False) + + # integration has more than one channel filter + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=False) + make_channel_filter(arc, is_default=False) + _assert(org, arc) + + # integration's default channel filter is setup to notify via slack but Slack is not configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=True) + _assert(org, arc, should_be_returned=False) + + # integration's default channel filter is setup to notify via slack and Slack is configured for the org + org, arc = _make_org_and_arc() slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() - assert organization.slack_is_configured is True - - -@pytest.mark.django_db -def test_telegram_is_configured(make_organization, make_telegram_channel): - organization = make_organization() - assert organization.telegram_is_configured is False - make_telegram_channel(organization) - assert organization.telegram_is_configured is True - - -@pytest.mark.django_db -def test_get_direct_paging_integrations(make_organization, make_team, make_alert_receive_channel): - org1 = make_organization() - org1_team1 = make_team(org1) - org1_team2 = make_team(org1) - - org2 = make_organization() - - org1_direct_paging_integration1 = make_alert_receive_channel( - org1, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=org1_team1 - ) - org1_direct_paging_integration2 = make_alert_receive_channel( - org1, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=org1_team2 + org.slack_team_identity = slack_team_identity + org.save() + + make_channel_filter(arc, is_default=True, notify_in_slack=True) + _assert(org, arc) + + # integration's default channel filter is setup to notify via telegram but Telegram is not configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) + _assert(org, arc, should_be_returned=False) + + # integration's default channel filter is setup to notify via telegram and Telegram is configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) + make_telegram_channel(org) + _assert(org, arc) + + # integration's default channel filter is contactable via a custom messaging backend + org, arc = _make_org_and_arc() + make_channel_filter( + arc, + is_default=True, + notify_in_slack=False, + notification_backends={"MSTEAMS": {"channel": "test", "enabled": True}}, ) + _assert(org, arc) - make_alert_receive_channel(org1, integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER) - make_alert_receive_channel(org2, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) - - org1_direct_paging_integrations = org1.get_direct_paging_integrations() - org2_direct_paging_integrations = org2.get_direct_paging_integrations() - - assert len(org1_direct_paging_integrations) == 2 - assert len(org2_direct_paging_integrations) == 1 - - assert org1_direct_paging_integration1 in org1_direct_paging_integrations - assert org1_direct_paging_integration2 in org1_direct_paging_integrations + # integration's default channel filter has an escalation chain attached to it + org, arc = _make_org_and_arc() + escalation_chain = make_escalation_chain(org) + make_channel_filter(arc, is_default=True, notify_in_slack=False, escalation_chain=escalation_chain) + _assert(org, arc) diff --git a/engine/common/api_helpers/paginators.py b/engine/common/api_helpers/paginators.py index 34a20c273b..1e1a202848 100644 --- a/engine/common/api_helpers/paginators.py +++ b/engine/common/api_helpers/paginators.py @@ -82,4 +82,4 @@ class FifteenPageSizePaginator(PathPrefixedPagePagination): class TwentyFiveCursorPaginator(PathPrefixedCursorPagination): page_size = 25 - ordering = "-pk" + ordering = "-started_at" diff --git a/engine/engine/tests/test_views.py b/engine/engine/tests/test_views.py new file mode 100644 index 0000000000..e462c33abc --- /dev/null +++ b/engine/engine/tests/test_views.py @@ -0,0 +1,42 @@ +import sys +from importlib import import_module, reload +from unittest.mock import patch + +import pytest +from django.conf import settings +from django.urls import clear_url_caches +from rest_framework import status +from rest_framework.test import APIClient + + +def reload_urlconf(): + clear_url_caches() + if settings.ROOT_URLCONF in sys.modules: + reload(sys.modules[settings.ROOT_URLCONF]) + return import_module(settings.ROOT_URLCONF) + + +@pytest.mark.skip(reason="TODO: This test is currently failing in oncall-private, skipping to unblock release") +@pytest.mark.parametrize( + "detached_integrations,urlconf,is_cache_updated", + [ + (False, None, True), + (True, None, False), + (True, "engine.integrations_urls", True), + ], +) +def test_startupprobe_populates_integrations_cache(settings, detached_integrations, urlconf, is_cache_updated): + settings.DETACHED_INTEGRATIONS_SERVER = detached_integrations + if urlconf: + settings.ROOT_URLCONF = urlconf + reload_urlconf() + + client = APIClient() + + with patch( + "apps.integrations.mixins.AlertChannelDefiningMixin.update_alert_receive_channel_cache" + ) as mock_update_cache: + response = client.get("/startupprobe/") + + assert response.status_code == status.HTTP_200_OK + assert mock_update_cache.called == is_cache_updated diff --git a/engine/engine/urls.py b/engine/engine/urls.py index c86a9a74c3..ff2d1c6d55 100644 --- a/engine/engine/urls.py +++ b/engine/engine/urls.py @@ -15,6 +15,7 @@ """ from django.conf import settings from django.conf.urls.static import static +from django.contrib import admin from django.urls import URLPattern, URLResolver, include, path from .views import HealthCheckView, MaintenanceModeStatusView, ReadinessCheckView, StartupProbeView @@ -76,7 +77,11 @@ ] + urlpatterns if settings.SILK_PROFILER_ENABLED: - urlpatterns += [path(settings.SILK_PATH, include("silk.urls", namespace="silk"))] + urlpatterns += [ + # need django admin enabled to be able to access silk + path(settings.ONCALL_DJANGO_ADMIN_PATH, admin.site.urls), + path(settings.SILK_PATH, include("silk.urls", namespace="silk")), + ] if settings.DRF_SPECTACULAR_ENABLED: from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView diff --git a/engine/engine/views.py b/engine/engine/views.py index 7f3e2f5d6c..0022d02f50 100644 --- a/engine/engine/views.py +++ b/engine/engine/views.py @@ -1,3 +1,4 @@ +from django import urls from django.conf import settings from django.core.cache import cache from django.http import HttpResponse, JsonResponse @@ -43,7 +44,13 @@ class StartupProbeView(View): dangerously_bypass_middlewares = True def get(self, request): - if cache.get(AlertChannelDefiningMixin.CACHE_KEY_DB_FALLBACK) is None: + # enable integrations cache if current engine instance is serving them + integrations_enabled = True + if settings.DETACHED_INTEGRATIONS_SERVER: + url_resolver = urls.get_resolver(urls.get_urlconf()) + integrations_enabled = url_resolver.namespace_dict.get("integrations") + + if integrations_enabled and cache.get(AlertChannelDefiningMixin.CACHE_KEY_DB_FALLBACK) is None: AlertChannelDefiningMixin().update_alert_receive_channel_cache() cache.set("healthcheck", "healthcheck", 30) # Checking cache connectivity diff --git a/engine/requirements.txt b/engine/requirements.txt index e6ac5e7332..87aabd8863 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -1,4 +1,4 @@ -django==4.2.6 +django==4.2.7 djangorestframework==3.14.0 slack_sdk==3.21.3 whitenoise==5.3.0 diff --git a/engine/settings/base.py b/engine/settings/base.py index 100ff125f1..cd58cb26ed 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -11,7 +11,6 @@ VERSION = "dev-oss" SEND_ANONYMOUS_USAGE_STATS = getenv_boolean("SEND_ANONYMOUS_USAGE_STATS", default=True) -ADMIN_ENABLED = False # disable django admin panel # License is OpenSource or Cloud OPEN_SOURCE_LICENSE_NAME = "OpenSource" @@ -590,6 +589,10 @@ class BrokerTypes: SILK_PROFILER_ENABLED = getenv_boolean("SILK_PROFILER_ENABLED", default=False) and not IS_IN_MAINTENANCE_MODE +# django admin panel is required to auth with django silk. Otherwise if silk isn't enabled, we don't need it. +ONCALL_DJANGO_ADMIN_PATH = os.environ.get("ONCALL_DJANGO_ADMIN_PATH", "django-admin") + "/" +ADMIN_ENABLED = SILK_PROFILER_ENABLED + if SILK_PROFILER_ENABLED: SILK_PATH = os.environ.get("SILK_PATH", "silk/") SILKY_INTERCEPT_PERCENT = getenv_integer("SILKY_INTERCEPT_PERCENT", 100) diff --git a/grafana-plugin/src/containers/AddResponders/AddResponders.tsx b/grafana-plugin/src/containers/AddResponders/AddResponders.tsx index b7249d4e5c..515bbe7ac1 100644 --- a/grafana-plugin/src/containers/AddResponders/AddResponders.tsx +++ b/grafana-plugin/src/containers/AddResponders/AddResponders.tsx @@ -11,7 +11,7 @@ import Text from 'components/Text/Text'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; import { Alert as AlertType } from 'models/alertgroup/alertgroup.types'; import { getTimezone } from 'models/user/user.helpers'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; import { useStore } from 'state/useStore'; import { UserActions } from 'utils/authorization'; @@ -62,7 +62,7 @@ const AddResponders = observer( const currentMoment = useMemo(() => dayjs(), []); const isCreateMode = mode === 'create'; - const [currentlyConsideredUser, setCurrentlyConsideredUser] = useState(null); + const [currentlyConsideredUser, setCurrentlyConsideredUser] = useState(null); const [currentlyConsideredUserNotificationPolicy, setCurrentlyConsideredUserNotificationPolicy] = useState(NotificationPolicyValue.Default); @@ -141,7 +141,7 @@ const AddResponders = observer( disableNotificationPolicySelect handleDelete={generateRemovePreviouslyPagedUserCallback(user.pk)} important={user.important} - data={user as unknown as User} + data={user as unknown as UserCurrentlyOnCall} /> ))} {selectedUserResponders.map((responder, index) => ( diff --git a/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts b/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts index a53fbf5d08..9fe481cf84 100644 --- a/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts +++ b/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts @@ -1,7 +1,7 @@ import { SelectableValue } from '@grafana/data'; import { ActionMeta } from '@grafana/ui'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; export enum NotificationPolicyValue { Default = 0, @@ -9,7 +9,7 @@ export enum NotificationPolicyValue { } export type UserResponder = { - data: User; + data: UserCurrentlyOnCall; important: boolean; }; export type UserResponders = UserResponder[]; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss index fbbd2163d3..7b5f57ac4a 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss @@ -30,6 +30,10 @@ margin: 8px; } +.loading-placeholder { + margin: 8px; +} + .table { max-height: 150px; overflow: auto; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx index af0ec1c9f3..37128060ef 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx @@ -21,7 +21,7 @@ describe('AddRespondersPopup', () => { }, ]; - test('it renders teams properly', () => { + test('it shows a loading message initially', () => { const mockStoreValue = { directPagingStore: { selectedTeamResponder: null, @@ -30,36 +30,7 @@ describe('AddRespondersPopup', () => { getSearchResult: jest.fn().mockReturnValue(teams), }, userStore: { - getSearchResult: jest.fn().mockReturnValue({ results: [] }), - }, - }; - - const component = render( - - - - ); - - expect(component.container).toMatchSnapshot(); - }); - - test('if a team is selected it shows an info alert', () => { - const mockStoreValue = { - directPagingStore: { - selectedTeamResponder: teams[0], - selectedUserResponders: [], - }, - grafanaTeamStore: { - getSearchResult: jest.fn().mockReturnValue(teams), - }, - userStore: { - getSearchResult: jest.fn().mockReturnValue({ results: [] }), + search: jest.fn().mockReturnValue({ results: [] }), }, }; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx index 9be2456d48..7a50007607 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx @@ -1,6 +1,6 @@ import React, { useState, useCallback, useEffect, useRef, FC } from 'react'; -import { Alert, HorizontalGroup, Icon, Input, RadioButtonGroup } from '@grafana/ui'; +import { Alert, HorizontalGroup, Icon, Input, LoadingPlaceholder, RadioButtonGroup } from '@grafana/ui'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { ColumnsType } from 'rc-table/lib/interface'; @@ -10,7 +10,7 @@ import GTable from 'components/GTable/GTable'; import Text from 'components/Text/Text'; import { Alert as AlertType } from 'models/alertgroup/alertgroup.types'; import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; import { useStore } from 'state/useStore'; import { useDebouncedCallback, useOnClickOutside } from 'utils/hooks'; @@ -21,7 +21,7 @@ type Props = { visible: boolean; setVisible: (value: boolean) => void; - setCurrentlyConsideredUser: (user: User) => void; + setCurrentlyConsideredUser: (user: UserCurrentlyOnCall) => void; setShowUserConfirmationModal: (value: boolean) => void; existingPagedUsers?: AlertType['paged_users']; @@ -34,13 +34,6 @@ enum TabOptions { Users = 'users', } -/** - * TODO: properly filter out 'No team'. Right now it shows up on first render and then shortly thereafter the component - * re-renders with 'No team' filtered out - * - * TODO: properly fetch/show loading state when fetching users. Right now it shows an empty list on the initial network - * request, we can probably have a better experience here - */ const AddRespondersPopup = observer( ({ mode, @@ -55,23 +48,13 @@ const AddRespondersPopup = observer( const isCreateMode = mode === 'create'; + const [searchLoading, setSearchLoading] = useState(true); const [activeOption, setActiveOption] = useState(isCreateMode ? TabOptions.Teams : TabOptions.Users); + const [teamSearchResults, setTeamSearchResults] = useState([]); + const [userSearchResults, setUserSearchResults] = useState([]); const [searchTerm, setSearchTerm] = useState(''); const ref = useRef(); - const teamSearchResults = grafanaTeamStore.getSearchResult(); - - let userSearchResults = userStore.getSearchResult().results || []; - - /** - * in the context where some user(s) have already been paged (ex. on a direct paging generated - * alert group detail page), we should filter out the search results to not include these users - */ - if (existingPagedUsers.length > 0) { - const existingPagedUserIds = existingPagedUsers.map(({ pk }) => pk); - userSearchResults = userSearchResults.filter(({ pk }) => !existingPagedUserIds.includes(pk)); - } - const usersCurrentlyOnCall = userSearchResults.filter(({ is_currently_oncall }) => is_currently_oncall); const usersNotCurrentlyOnCall = userSearchResults.filter(({ is_currently_oncall }) => !is_currently_oncall); @@ -87,7 +70,7 @@ const AddRespondersPopup = observer( ); const onClickUser = useCallback( - async (user: User) => { + async (user: UserCurrentlyOnCall) => { if (isCreateMode && user.is_currently_oncall) { directPagingStore.addUserToSelectedUsers(user); } else { @@ -113,18 +96,86 @@ const AddRespondersPopup = observer( [setVisible, directPagingStore, setActiveOption] ); - const handleSearchTermChange = useDebouncedCallback(() => { + const searchForUsers = useCallback(async () => { + const userResults = await userStore.search({ searchTerm, is_currently_oncall: 'all' }); + setUserSearchResults(userResults.results); + }, [searchTerm]); + + const searchForTeams = useCallback(async () => { + await grafanaTeamStore.updateItems(searchTerm, false, true, false); + setTeamSearchResults(grafanaTeamStore.getSearchResult()); + }, [searchTerm]); + + const handleSearchTermChange = useDebouncedCallback(async () => { + setSearchLoading(true); + if (isCreateMode && activeOption === TabOptions.Teams) { - grafanaTeamStore.updateItems(searchTerm, false, true, false); + await searchForTeams(); } else { - userStore.updateItems({ searchTerm, short: 'false' }); + await searchForUsers(); } + + setSearchLoading(false); }, 500); - useEffect(handleSearchTermChange, [searchTerm, activeOption]); + const onChangeTab = useCallback( + async (tab: TabOptions) => { + /** + * there's no need to trigger a new search request when the user changes tabs if they don't have a + * search term + */ + if (searchTerm) { + setSearchLoading(true); + + if (activeOption === TabOptions.Teams) { + await searchForTeams(); + } else { + await searchForUsers(); + } + + setSearchLoading(false); + } + + setActiveOption(tab); + }, + [searchTerm] + ); + + useEffect(handleSearchTermChange, [searchTerm]); + + /** + * in the context where some user(s) have already been paged (ex. on a direct paging generated + * alert group detail page), we should filter out the search results to not include these users + */ + useEffect(() => { + if (existingPagedUsers.length > 0) { + const existingPagedUserIds = existingPagedUsers.map(({ pk }) => pk); + setUserSearchResults((userSearchResults) => + userSearchResults.filter(({ pk }) => !existingPagedUserIds.includes(pk)) + ); + } + }, [existingPagedUsers]); + + /** + * pre-populate the users and teams search results so that when the user opens AddRespondersPopup it is already + * populated with data (nicer UX) + */ + useEffect(() => { + (async () => { + /** + * teams are not relevant when the component is rendered in "update" mode so we skip fetching teams here + */ + if (isCreateMode) { + await searchForTeams(); + } + + await searchForUsers(); + setSearchLoading(false); + })(); + }, []); const userIsSelected = useCallback( - (user: User) => selectedUserResponders.some((userResponder) => userResponder.data.pk === user.pk), + (user: UserCurrentlyOnCall) => selectedUserResponders.some((userResponder) => userResponder.data.pk === user.pk), [selectedUserResponders] ); @@ -155,11 +206,11 @@ const AddRespondersPopup = observer( }, ]; - const userColumns: ColumnsType = [ + const userColumns: ColumnsType = [ // TODO: how to make the rows span full width properly? { width: 300, - render: (user: User) => { + render: (user: UserCurrentlyOnCall) => { const { avatar, name, username, teams } = user; const disabled = userIsSelected(user); @@ -170,6 +221,7 @@ const AddRespondersPopup = observer( {name || username} + {/* TODO: we should add an elippsis and/or tooltip in the event that the user has a ton of teams */} {teams?.length > 0 && {teams.map(({ name }) => name).join(', ')}} @@ -179,18 +231,18 @@ const AddRespondersPopup = observer( }, { width: 40, - render: (user: User) => (userIsSelected(user) ? : null), + render: (user: UserCurrentlyOnCall) => (userIsSelected(user) ? : null), key: 'Checked', }, ]; - const UserResultsSection: FC<{ header: string; users: User[] }> = ({ header, users }) => + const UserResultsSection: FC<{ header: string; users: UserCurrentlyOnCall[] }> = ({ header, users }) => users.length > 0 && ( <> {header} - + emptyText={users ? 'No users found' : 'Loading...'} rowKey="pk" columns={userColumns} @@ -223,11 +275,12 @@ const AddRespondersPopup = observer( ]} className={cx('radio-buttons')} value={activeOption} - onChange={setActiveOption} + onChange={onChangeTab} fullWidth /> )} - {activeOption === TabOptions.Teams && ( + {searchLoading && } + {!searchLoading && activeOption === TabOptions.Teams && ( <> {selectedTeamResponder ? ( )} - {activeOption === TabOptions.Users && ( + {!searchLoading && activeOption === TabOptions.Users && ( <> diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap index 16a3c3c9dc..ba89c5668a 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap @@ -1,119 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`AddRespondersPopup if a team is selected it shows an info alert 1`] = ` -
-
-
-
- -
-
- - - -
-
-
-
-
- - - - -
-
-
-
- - - -
-
-
-
- You can add only one team per escalation. Please remove the existing team before adding a new one. -
-
-
-
-
-`; - -exports[`AddRespondersPopup it renders teams properly 1`] = ` +exports[`AddRespondersPopup it shows a loading message initially 1`] = `
+ Loading... +
-
- - - -
-
-
-
- - You can only page teams which have a Direct Paging integration that is configured. - - - -
-
- Learn more -
-
-
- - - -
-
-
- - - -
-
-
-
-
-
-
- - - - - - - - - - - - -
-
-
-
-
-
- -
-
- - my test team - -
-
-
-
- - 1 - user - - on-call - -
-
-
-
-
-
-
-
-
- -
-
- - my test team 2 - -
-
-
-
-
-
-
-
+