Something foo + baz
", + "image_url": "http://foo", + "source_link": None, + }, + } + assert response.json() == expected + # enable labels feature flag + settings.FEATURE_LABELS_ENABLED_FOR_ALL = True + alert_group_with_labels = make_alert_group(alert_receive_channel) + alert_with_labels = make_alert(alert_group_with_labels, alert_payload) + _ = make_alert_group_label_association( + organization, alert_group_with_labels, key_name=SERVICE_LABEL, value_name=METRICS_TEST_SERVICE_NAME + ) + + url = reverse( + "api-gi:alert-groups-detail", kwargs={"public_primary_key": alert_group_with_labels.public_primary_key} + ) + response = client.get(url, format="json", **headers) + expected = { + "id": alert_group_with_labels.public_primary_key, + "link": alert_group_with_labels.web_link, + "status": "new", + "title": alert_group_with_labels.long_verbose_name_without_formatting, + "alerts": [ + { + "id_oncall": alert_with_labels.public_primary_key, + "payload": alert_payload, + } + ], + "labels": [ + { + "key": "service_name", + "value": "test_service", + } + ], "render_for_web": { "title": "title: bar", "message": "Something foo + baz
", diff --git a/engine/apps/base/models/user_notification_policy.py b/engine/apps/base/models/user_notification_policy.py index df854a82..1593b3ca 100644 --- a/engine/apps/base/models/user_notification_policy.py +++ b/engine/apps/base/models/user_notification_policy.py @@ -5,12 +5,11 @@ from typing import Tuple from django.conf import settings from django.core.exceptions import ValidationError from django.core.validators import MinLengthValidator -from django.db import IntegrityError, models +from django.db import models from django.db.models import Q from apps.base.messaging import get_messaging_backends from apps.user_management.models import User -from common.exceptions import UserNotificationPolicyCouldNotBeDeleted from common.ordered_model.ordered_model import OrderedModel from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -66,32 +65,7 @@ def validate_channel_choice(value): raise ValidationError("%(value)s is not a valid option", params={"value": value}) -class UserNotificationPolicyQuerySet(models.QuerySet): - def create_default_policies_for_user(self, user: User) -> None: - if user.notification_policies.filter(important=False).exists(): - return - - policies_to_create = user.default_notification_policies_defaults - - try: - super().bulk_create(policies_to_create) - except IntegrityError: - pass - - def create_important_policies_for_user(self, user: User) -> None: - if user.notification_policies.filter(important=True).exists(): - return - - policies_to_create = user.important_notification_policies_defaults - - try: - super().bulk_create(policies_to_create) - except IntegrityError: - pass - - class UserNotificationPolicy(OrderedModel): - objects = UserNotificationPolicyQuerySet.as_manager() order_with_respect_to = ("user_id", "important") public_primary_key = models.CharField( @@ -171,12 +145,6 @@ class UserNotificationPolicy(OrderedModel): else: return "Not set" - def delete(self): - if UserNotificationPolicy.objects.filter(important=self.important, user=self.user).count() == 1: - raise UserNotificationPolicyCouldNotBeDeleted("Can't delete last user notification policy") - else: - super().delete() - class NotificationChannelOptions: """ diff --git a/engine/apps/base/tests/test_user_notification_policy.py b/engine/apps/base/tests/test_user_notification_policy.py index 41354491..7997f2bd 100644 --- a/engine/apps/base/tests/test_user_notification_policy.py +++ b/engine/apps/base/tests/test_user_notification_policy.py @@ -9,7 +9,6 @@ from apps.base.models.user_notification_policy import ( validate_channel_choice, ) from apps.base.tests.messaging_backend import TestOnlyBackend -from common.exceptions import UserNotificationPolicyCouldNotBeDeleted @pytest.mark.parametrize( @@ -84,7 +83,7 @@ def test_extra_messaging_backends_details(): @pytest.mark.django_db -def test_unable_to_delete_last_notification_policy( +def test_can_delete_last_notification_policy( make_organization, make_user_for_organization, make_user_notification_policy, @@ -101,5 +100,4 @@ def test_unable_to_delete_last_notification_policy( ) first_policy.delete() - with pytest.raises(UserNotificationPolicyCouldNotBeDeleted): - second_policy.delete() + second_policy.delete() diff --git a/engine/apps/chatops_proxy/client.py b/engine/apps/chatops_proxy/client.py index baa382d5..1a0b63bc 100644 --- a/engine/apps/chatops_proxy/client.py +++ b/engine/apps/chatops_proxy/client.py @@ -65,7 +65,7 @@ class ChatopsProxyAPIClient: # OnCall Tenant def register_tenant( - self, service_tenant_id: str, cluster_slug: str, service_type: str, stack_id: int + self, service_tenant_id: str, cluster_slug: str, service_type: str, stack_id: int, stack_slug: str ) -> tuple[Tenant, requests.models.Response]: url = f"{self.api_base_url}/tenants/register" d = { @@ -74,6 +74,7 @@ class ChatopsProxyAPIClient: "cluster_slug": cluster_slug, "service_type": service_type, "stack_id": stack_id, + "stack_slug": stack_slug, } } response = requests.post(url=url, json=d, headers=self._headers) @@ -170,6 +171,22 @@ class ChatopsProxyAPIClient: self._check_response(response) return OAuthInstallation(**response.json()["oauth_installation"]), response + def delete_oauth_installation( + self, + stack_id: int, + provider_type: str, + grafana_user_id: int, + ) -> tuple[bool, requests.models.Response]: + url = f"{self.api_base_url}/oauth_installations/uninstall" + d = { + "stack_id": stack_id, + "provider_type": provider_type, + "grafana_user_id": grafana_user_id, + } + response = requests.post(url=url, json=d, headers=self._headers) + self._check_response(response) + return response.json()["removed"], response + def _check_response(self, response: requests.models.Response): """ Wraps an exceptional response to ChatopsProxyAPIException diff --git a/engine/apps/chatops_proxy/register_oncall_tenant.py b/engine/apps/chatops_proxy/register_oncall_tenant.py new file mode 100644 index 00000000..5aaa45c4 --- /dev/null +++ b/engine/apps/chatops_proxy/register_oncall_tenant.py @@ -0,0 +1,18 @@ +# register_oncall_tenant moved to separate file from engine/apps/chatops_proxy/utils.py to avoid circular imports. +from django.conf import settings + +from apps.chatops_proxy.client import SERVICE_TYPE_ONCALL, ChatopsProxyAPIClient + + +def register_oncall_tenant(org): + """ + register_oncall_tenant registers oncall organization as a tenant in chatops-proxy. + """ + client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) + client.register_tenant( + str(org.uuid), + settings.ONCALL_BACKEND_REGION, + SERVICE_TYPE_ONCALL, + org.stack_id, + org.stack_slug, + ) diff --git a/engine/apps/chatops_proxy/tasks.py b/engine/apps/chatops_proxy/tasks.py index bbe3dcd0..785233e3 100644 --- a/engine/apps/chatops_proxy/tasks.py +++ b/engine/apps/chatops_proxy/tasks.py @@ -1,9 +1,12 @@ +from functools import partial + from celery.utils.log import get_task_logger from django.conf import settings from common.custom_celery_tasks import shared_dedicated_queue_retry_task from .client import ChatopsProxyAPIClient, ChatopsProxyAPIException +from .register_oncall_tenant import register_oncall_tenant task_logger = get_task_logger(__name__) @@ -18,26 +21,42 @@ def register_oncall_tenant_async(**kwargs): cluster_slug = kwargs.get("cluster_slug") service_type = kwargs.get("service_type") stack_id = kwargs.get("stack_id") + stack_slug = kwargs.get("stack_slug") + org_id = kwargs.get("org_id") - client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) - try: - client.register_tenant(service_tenant_id, cluster_slug, service_type, stack_id) - except ChatopsProxyAPIException as api_exc: - task_logger.error( - f'msg="Failed to register OnCall tenant: {api_exc.msg}" service_tenant_id={service_tenant_id} cluster_slug={cluster_slug}' + # Temporary hack to support both old and new set of arguments + if org_id: + from apps.user_management.models import Organization + + try: + org = Organization.objects.get(pk=org_id) + except Organization.DoesNotExist: + task_logger.info(f"register_oncall_tenant_async: organization {org_id} was not found") + return + register_func = partial(register_oncall_tenant, org) + else: + client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) + register_func = partial( + client.register_tenant, service_tenant_id, cluster_slug, service_type, stack_id, stack_slug ) + try: + register_func() + except ChatopsProxyAPIException as api_exc: + # TODO: remove this check once new upsert tenant api is released if api_exc.status == 409: # 409 Indicates that it's impossible to register tenant, because tenant already registered. # Not retrying in this case, because manual conflict-resolution needed. + task_logger.info(f"register_oncall_tenant_async: tenant for organization {org_id} already exists") return else: # Otherwise keep retrying task + task_logger.error( + f"register_oncall_tenant_async: failed to register tenant for organization {org_id}: {api_exc.msg}" + ) raise api_exc except Exception as e: # Keep retrying task for any other exceptions too - task_logger.error( - f"Failed to register OnCall tenant: {e} service_tenant_id={service_tenant_id} cluster_slug={cluster_slug}" - ) + task_logger.error(f"register_oncall_tenant_async: failed to register tenant for organization {org_id}: {e}") raise e @@ -120,3 +139,48 @@ def unlink_slack_team_async(**kwargs): f'msg="Failed to unlink slack_team: {e}" service_tenant_id={service_tenant_id} slack_team_id={slack_team_id}' ) raise e + + +@shared_dedicated_queue_retry_task( + autoretry_for=(Exception,), + retry_backoff=True, + max_retries=0, +) +def start_sync_org_with_chatops_proxy(): + from apps.user_management.models import Organization + + organization_qs = Organization.objects.all() + organization_pks = organization_qs.values_list("pk", flat=True) + + max_countdown = 60 * 30 # 30 minutes, feel free to adjust + for idx, organization_pk in enumerate(organization_pks): + countdown = idx % max_countdown + sync_org_with_chatops_proxy.apply_async(kwargs={"org_id": organization_pk}, countdown=countdown) + + +@shared_dedicated_queue_retry_task( + autoretry_for=(Exception,), + retry_backoff=True, + max_retries=3, +) +def sync_org_with_chatops_proxy(**kwargs): + from apps.user_management.models import Organization + + org_id = kwargs.get("org_id") + task_logger.info(f"sync_org_with_chatops_proxy: started org_id={org_id}") + + try: + org = Organization.objects.get(pk=org_id) + except Organization.DoesNotExist: + task_logger.info(f"sync_org_with_chatops_proxy: organization {org_id} was not found") + return + + try: + register_oncall_tenant(org) + except ChatopsProxyAPIException as api_exc: + # TODO: once tenants upsert api is released, remove this check + if api_exc.status == 409: + task_logger.info(f"sync_org_with_chatops_proxy: tenant for organization {org_id} already exists") + # 409 Indicates that it's impossible to register tenant, because tenant already registered. + return + raise api_exc diff --git a/engine/apps/chatops_proxy/utils.py b/engine/apps/chatops_proxy/utils.py index 60d70391..06e6f14e 100644 --- a/engine/apps/chatops_proxy/utils.py +++ b/engine/apps/chatops_proxy/utils.py @@ -7,6 +7,7 @@ import typing from django.conf import settings from .client import PROVIDER_TYPE_SLACK, SERVICE_TYPE_ONCALL, ChatopsProxyAPIClient, ChatopsProxyAPIException +from .register_oncall_tenant import register_oncall_tenant from .tasks import ( link_slack_team_async, register_oncall_tenant_async, @@ -48,31 +49,24 @@ def get_slack_oauth_response_from_chatops_proxy(stack_id) -> dict: return slack_installation.oauth_response -def register_oncall_tenant(service_tenant_id: str, cluster_slug: str, stack_id: int): +def register_oncall_tenant_with_async_fallback(org): """ register_oncall_tenant tries to register oncall tenant synchronously and fall back to task in case of any exceptions to make sure that tenant is registered. First attempt is synchronous to register tenant ASAP to not miss any chatops requests. """ - client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) try: - client.register_tenant( - service_tenant_id, - cluster_slug, - SERVICE_TYPE_ONCALL, - stack_id, - ) + register_oncall_tenant(org) except Exception as e: - logger.error( - f"create_oncall_connector: failed " - f"oncall_org_id={service_tenant_id} backend={cluster_slug} stack_id={stack_id} exc={e}" - ) + logger.error(f"create_oncall_connector: failed organization_id={org} exc={e}") register_oncall_tenant_async.apply_async( kwargs={ - "service_tenant_id": service_tenant_id, - "cluster_slug": cluster_slug, + "service_tenant_id": str(org.uuid), + "cluster_slug": settings.ONCALL_BACKEND_REGION, "service_type": SERVICE_TYPE_ONCALL, - "stack_id": stack_id, + "stack_id": org.stack_id, + "stack_slug": org.stack_slug, + "org_id": org.id, }, countdown=2, ) @@ -141,3 +135,23 @@ def unlink_slack_team(service_tenant_id: str, slack_team_id: str): "service_type": SERVICE_TYPE_ONCALL, } ) + + +def uninstall_slack(stack_id: int, grafana_user_id: int) -> bool: + """ + uninstall_slack uninstalls slack integration from chatops-proxy and returns bool indicating if it was removed. + If such installation does not exist - returns True as well. + """ + client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) + try: + removed, response = client.delete_oauth_installation(stack_id, PROVIDER_TYPE_SLACK, grafana_user_id) + except ChatopsProxyAPIException as api_exc: + if api_exc.status == 404: + return True + logger.exception( + "uninstall_slack: error trying to install slack from chatops-proxy: " "error=%s", + api_exc, + ) + return False + + return removed diff --git a/engine/apps/phone_notifications/phone_backend.py b/engine/apps/phone_notifications/phone_backend.py index a8cf50d6..c3db1094 100644 --- a/engine/apps/phone_notifications/phone_backend.py +++ b/engine/apps/phone_notifications/phone_backend.py @@ -377,7 +377,7 @@ class PhoneBackend: def _notify_connected_number(self, user): text = ( - f"This phone number has been connected to Grafana OnCall team" + f"This phone number has been connected to Grafana OnCall team " f'"{user.organization.stack_slug}"\nYour Grafana OnCall <3' ) try: @@ -392,7 +392,7 @@ class PhoneBackend: def _notify_disconnected_number(self, user, number): text = ( - f"This phone number has been disconnected from Grafana OnCall team" + f"This phone number has been disconnected from Grafana OnCall team " f'"{user.organization.stack_slug}"\nYour Grafana OnCall <3' ) try: diff --git a/engine/apps/public_api/tests/test_shift_swap.py b/engine/apps/public_api/tests/test_shift_swap.py index c4828d11..65af6a4f 100644 --- a/engine/apps/public_api/tests/test_shift_swap.py +++ b/engine/apps/public_api/tests/test_shift_swap.py @@ -80,7 +80,7 @@ def test_list_filters( def assert_expected(response, expected): assert response.status_code == status.HTTP_200_OK returned = [s["id"] for s in response.json().get("results", [])] - assert returned == [s.public_primary_key for s in expected] + assert sorted(returned) == sorted([s.public_primary_key for s in expected]) client = APIClient() base_url = reverse("api-public:shift_swap-list") diff --git a/engine/apps/public_api/views/personal_notifications.py b/engine/apps/public_api/views/personal_notifications.py index b1288b4b..44b251a3 100644 --- a/engine/apps/public_api/views/personal_notifications.py +++ b/engine/apps/public_api/views/personal_notifications.py @@ -12,7 +12,6 @@ from apps.user_management.models import User from common.api_helpers.exceptions import BadRequest from common.api_helpers.mixins import RateLimitHeadersMixin, UpdateSerializerMixin from common.api_helpers.paginators import FiftyPageSizePaginator -from common.exceptions import UserNotificationPolicyCouldNotBeDeleted from common.insight_log import EntityEvent, write_resource_insight_log @@ -75,10 +74,7 @@ class PersonalNotificationView(RateLimitHeadersMixin, UpdateSerializerMixin, Mod def perform_destroy(self, instance): user = self.request.user prev_state = user.insight_logs_serialized - try: - instance.delete() - except UserNotificationPolicyCouldNotBeDeleted: - raise BadRequest(detail="Can't delete last user notification policy") + instance.delete() new_state = user.insight_logs_serialized write_resource_insight_log( instance=user, diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index 22532dc0..b9191234 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -706,6 +706,13 @@ def _create_user_option_groups( } ) + # Only inject chatops-proxy metadata into the first dropdown option to reduce payload size + # so the 250kb Slack limit is not exceeded for orgs with many users + if option_groups: + option_groups[0]["options"][0]["value"] = make_value( + json.loads(option_groups[0]["options"][0]["value"]), organization + ) + return option_groups diff --git a/engine/apps/slack/urls.py b/engine/apps/slack/urls.py index c17e1115..ec7c4acb 100644 --- a/engine/apps/slack/urls.py +++ b/engine/apps/slack/urls.py @@ -1,5 +1,7 @@ from django.urls import path +from common.api_helpers.optional_slash_router import optional_slash_path + from .views import ( InstallLinkRedirectView, OAuthSlackView, @@ -13,8 +15,8 @@ urlpatterns = [ path("event_api_endpoint/", SlackEventApiEndpointView.as_view()), path("interactive_api_endpoint/", SlackEventApiEndpointView.as_view()), # New urls used in cloud via chatops-proxy v3. - path("events/", SlackEventApiEndpointView.as_view()), - path("interactive/", SlackEventApiEndpointView.as_view()), + optional_slash_path("events", SlackEventApiEndpointView.as_view()), + optional_slash_path("interactive", SlackEventApiEndpointView.as_view()), # Trailing / is missing here on purpose. QA the feature if you want to add it. No idea why doesn't it work with it. path("reset_slack", ResetSlackView.as_view(), name="reset-slack"), # Deprecated. diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index e921c609..9578bbc4 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -15,6 +15,7 @@ from rest_framework.views import APIView from apps.api.permissions import RBACPermission from apps.auth_token.auth import PluginAuthentication from apps.base.utils import live_settings +from apps.chatops_proxy.utils import uninstall_slack as uninstall_slack_from_chatops_proxy from apps.slack.client import SlackClient from apps.slack.errors import SlackAPIError from apps.slack.scenarios.alertgroup_appearance import STEPS_ROUTING as ALERTGROUP_APPEARANCE_ROUTING @@ -573,8 +574,19 @@ class ResetSlackView(APIView): "Grafana OnCall is temporary unable to connect your slack account or install OnCall to your slack workspace", status=400, ) + if settings.UNIFIED_SLACK_APP_ENABLED: + # If unified slack app is enabled - uninstall slack integration from chatops-proxy first and on success - + # uninstall it from OnCall. + removed = uninstall_slack_from_chatops_proxy(request.user.organization.stack_id, request.user.user_id) + else: + # just a placeholder value to continute uninstallation until UNIFIED_SLACK_APP_ENABLED is not enabled + removed = True + if not removed: + return Response({"error": "Failed to uninstall slack integration"}, status=500) + try: uninstall_slack_integration(request.user.organization, request.user) except SlackInstallationExc as e: return Response({"error": e.error_message}, status=400) + return Response(status=200) diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index e82b8ace..2faa2428 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -11,7 +11,11 @@ from django.utils import timezone from mirage import fields as mirage_fields from apps.alerts.models import MaintainableObject -from apps.chatops_proxy.utils import register_oncall_tenant, unlink_slack_team, unregister_oncall_tenant +from apps.chatops_proxy.utils import ( + register_oncall_tenant_with_async_fallback, + unlink_slack_team, + unregister_oncall_tenant, +) from apps.user_management.subscription_strategy import FreePublicBetaSubscriptionStrategy from apps.user_management.types import AlertGroupTableColumn from common.insight_log import ChatOpsEvent, ChatOpsTypePlug, write_chatops_insight_log @@ -61,7 +65,7 @@ class OrganizationQuerySet(models.QuerySet): def create(self, **kwargs): instance = super().create(**kwargs) if settings.FEATURE_MULTIREGION_ENABLED: - register_oncall_tenant(str(instance.uuid), settings.ONCALL_BACKEND_REGION, instance.stack_id) + register_oncall_tenant_with_async_fallback(instance) return instance def delete(self): diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index d121719e..b7d99200 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -9,7 +9,7 @@ import pytz from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.validators import MinLengthValidator -from django.db import models, transaction +from django.db import models from django.db.models import Q from django.db.models.signals import post_save from django.dispatch import receiver @@ -84,8 +84,6 @@ class UserManager(models.Manager["User"]): @staticmethod def sync_for_organization(organization, api_users: list[dict]): - from apps.base.models import UserNotificationPolicy - grafana_users = {user["userId"]: user for user in api_users} existing_user_ids = set(organization.users.all().values_list("user_id", flat=True)) @@ -105,21 +103,7 @@ class UserManager(models.Manager["User"]): if user["userId"] not in existing_user_ids ) - with transaction.atomic(): - organization.users.bulk_create(users_to_create, batch_size=5000) - # Retrieve primary keys for the newly created users - # - # If the model’s primary key is an AutoField, the primary key attribute can only be retrieved - # on certain databases (currently PostgreSQL, MariaDB 10.5+, and SQLite 3.35+). - # On other databases, it will not be set. - # https://docs.djangoproject.com/en/4.1/ref/models/querysets/#django.db.models.query.QuerySet.bulk_create - created_users = organization.users.exclude(user_id__in=existing_user_ids) - - policies_to_create = () - for user in created_users: - policies_to_create = policies_to_create + user.default_notification_policies_defaults - policies_to_create = policies_to_create + user.important_notification_policies_defaults - UserNotificationPolicy.objects.bulk_create(policies_to_create, batch_size=5000) + organization.users.bulk_create(users_to_create, batch_size=5000) # delete excess users user_ids_to_delete = existing_user_ids - grafana_users.keys() @@ -429,43 +413,25 @@ class User(models.Model): return PermissionsQuery(permissions__contains=[required_permission]) return RoleInQuery(role__lte=permission.fallback_role.value) - def get_or_create_notification_policies(self, important=False): + def get_notification_policies_or_use_default_fallback( + self, important=False + ) -> typing.List["UserNotificationPolicy"]: + """ + If the user has no notification policies defined, fallback to using e-mail as the notification channel. + """ + from apps.base.models import UserNotificationPolicy + if not self.notification_policies.filter(important=important).exists(): - if important: - self.notification_policies.create_important_policies_for_user(self) - else: - self.notification_policies.create_default_policies_for_user(self) - notification_policies = self.notification_policies.filter(important=important) - return notification_policies - - @property - def default_notification_policies_defaults(self): - from apps.base.models import UserNotificationPolicy - - print(self) - - return ( - UserNotificationPolicy( - user=self, - step=UserNotificationPolicy.Step.NOTIFY, - notify_by=settings.EMAIL_BACKEND_INTERNAL_ID, - order=0, - ), - ) - - @property - def important_notification_policies_defaults(self): - from apps.base.models import UserNotificationPolicy - - return ( - UserNotificationPolicy( - user=self, - step=UserNotificationPolicy.Step.NOTIFY, - notify_by=settings.EMAIL_BACKEND_INTERNAL_ID, - important=True, - order=0, - ), - ) + return [ + UserNotificationPolicy( + user=self, + step=UserNotificationPolicy.Step.NOTIFY, + notify_by=settings.EMAIL_BACKEND_INTERNAL_ID, + important=important, + order=0, + ), + ] + return list(self.notification_policies.filter(important=important).all()) def update_alert_group_table_selected_columns(self, columns: typing.List[AlertGroupTableColumn]) -> None: if self.alert_group_table_selected_columns != columns: @@ -498,9 +464,6 @@ class User(models.Model): # TODO: check whether this signal can be moved to save method of the model @receiver(post_save, sender=User) def listen_for_user_model_save(sender: User, instance: User, created: bool, *args, **kwargs) -> None: - if created: - instance.notification_policies.create_default_policies_for_user(instance) - instance.notification_policies.create_important_policies_for_user(instance) drop_cached_ical_for_custom_events_for_organization.apply_async( (instance.organization_id,), ) diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 7502557c..3cfd875a 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -118,18 +118,6 @@ def test_sync_users_for_organization(make_organization, make_user_for_organizati assert created_user.name == api_users[1]["name"] assert created_user.avatar_full_url == "https://test.test/test/1234" - assert created_user.notification_policies.filter(important=False).count() == 1 - assert ( - created_user.notification_policies.filter(important=False).first().notify_by - == settings.EMAIL_BACKEND_INTERNAL_ID - ) - - assert created_user.notification_policies.filter(important=True).count() == 1 - assert ( - created_user.notification_policies.filter(important=True).first().notify_by - == settings.EMAIL_BACKEND_INTERNAL_ID - ) - @pytest.mark.django_db def test_sync_users_for_organization_role_none(make_organization, make_user_for_organization): diff --git a/engine/common/exceptions/__init__.py b/engine/common/exceptions/__init__.py index ec543ab8..75622cd2 100644 --- a/engine/common/exceptions/__init__.py +++ b/engine/common/exceptions/__init__.py @@ -3,5 +3,4 @@ from .exceptions import ( # noqa: F401 MaintenanceCouldNotBeStartedError, TeamCanNotBeChangedError, UnableToSendDemoAlert, - UserNotificationPolicyCouldNotBeDeleted, ) diff --git a/engine/common/exceptions/exceptions.py b/engine/common/exceptions/exceptions.py index f176812b..9908e3bb 100644 --- a/engine/common/exceptions/exceptions.py +++ b/engine/common/exceptions/exceptions.py @@ -19,10 +19,6 @@ class UnableToSendDemoAlert(OperationCouldNotBePerformedError): pass -class UserNotificationPolicyCouldNotBeDeleted(OperationCouldNotBePerformedError): - pass - - class BacksyncIntegrationRequestError(Exception): """Error making request to alert receive channel backsync connection.""" diff --git a/engine/settings/base.py b/engine/settings/base.py index 66e6860b..d185d8bb 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -590,6 +590,14 @@ CELERY_BEAT_SCHEDULE = { }, } +START_SYNC_ORG_WITH_CHATOPS_PROXY_ENABLED = getenv_boolean("START_SYNC_ORG_WITH_CHATOPS_PROXY_ENABLED", default=False) +if FEATURE_MULTIREGION_ENABLED and START_SYNC_ORG_WITH_CHATOPS_PROXY_ENABLED: + CELERY_BEAT_SCHEDULE["start_sync_org_with_chatops_proxy"] = { + "task": "apps.chatops_proxy.tasks.start_sync_org_with_chatops_proxy", + "schedule": crontab(hour="*/24"), # Every 24 hours, feel free to adjust + "args": (), + } + if ESCALATION_AUDITOR_ENABLED: CELERY_BEAT_SCHEDULE["check_escalations"] = { "task": "apps.alerts.tasks.check_escalation_finished.check_escalation_finished_task", diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index c4ffa5ae..01d4a155 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -91,6 +91,8 @@ CELERY_TASK_ROUTES = { "apps.chatops_proxy.tasks.unlink_slack_team_async": {"queue": "default"}, "apps.chatops_proxy.tasks.register_oncall_tenant_async": {"queue": "default"}, "apps.chatops_proxy.tasks.unregister_oncall_tenant_async": {"queue": "default"}, + "apps.chatops_proxy.tasks.start_sync_org_with_chatops_proxy": {"queue": "default"}, + "apps.chatops_proxy.tasks.sync_org_with_chatops_proxy": {"queue": "default"}, # CRITICAL "apps.alerts.tasks.acknowledge_reminder.acknowledge_reminder_task": {"queue": "critical"}, "apps.alerts.tasks.acknowledge_reminder.unacknowledge_timeout_task": {"queue": "critical"}, diff --git a/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts b/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts index f2e8d0f1..6859ec03 100644 --- a/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts +++ b/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts @@ -3,7 +3,7 @@ import { verifyThatAlertGroupIsTriggered } from '../utils/alertGroup'; import { createEscalationChain, EscalationStep } from '../utils/escalationChain'; import { generateRandomValue } from '../utils/forms'; import { createIntegrationAndSendDemoAlert } from '../utils/integrations'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('we can create an oncall schedule + receive an alert', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; @@ -11,7 +11,7 @@ test('we can create an oncall schedule + receive an alert', async ({ adminRolePa const integrationName = generateRandomValue(); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await createEscalationChain( page, escalationChainName, diff --git a/grafana-plugin/e2e-tests/insights/insights.test.ts b/grafana-plugin/e2e-tests/insights/insights.test.ts index 14d2feec..411a8cd4 100644 --- a/grafana-plugin/e2e-tests/insights/insights.test.ts +++ b/grafana-plugin/e2e-tests/insights/insights.test.ts @@ -6,7 +6,7 @@ import { createEscalationChain, EscalationStep } from '../utils/escalationChain' import { clickButton, generateRandomValue } from '../utils/forms'; import { createIntegrationAndSendDemoAlert } from '../utils/integrations'; import { goToGrafanaPage, goToOnCallPage } from '../utils/navigation'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; /** * Insights is dependent on Scenes which were only added in Grafana 10.0.0 @@ -66,7 +66,7 @@ test.describe.skip('Insights', () => { const escalationChainName = generateRandomValue(); const integrationName = generateRandomValue(); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await createEscalationChain( page, escalationChainName, diff --git a/grafana-plugin/e2e-tests/schedules/addOverride.test.ts b/grafana-plugin/e2e-tests/schedules/addOverride.test.ts index 709c8551..a741c933 100644 --- a/grafana-plugin/e2e-tests/schedules/addOverride.test.ts +++ b/grafana-plugin/e2e-tests/schedules/addOverride.test.ts @@ -1,14 +1,16 @@ import dayjs from 'dayjs'; -import { test, expect } from '../fixtures'; +import { test, expect, Locator } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; import { clickButton, generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation, getOverrideFormDateInputs } from '../utils/schedule'; +import { setTimezoneInProfile } from '../utils/grafanaProfile'; +import { createOnCallSchedule, getOverrideFormDateInputs } from '../utils/schedule'; -test('default dates in override creation modal are correct', async ({ adminRolePage }) => { +test('Default dates in override creation modal are set to today', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await clickButton({ page, buttonText: 'Add override' }); @@ -20,3 +22,39 @@ test('default dates in override creation modal are correct', async ({ adminRoleP expect(overrideFormDateInputs.start.isSame(expectedStart)).toBe(true); expect(overrideFormDateInputs.end.isSame(expectedEnd)).toBe(true); }); + +test('Fills in override time and reacts to timezone change', async ({ adminRolePage }) => { + const { page, userName } = adminRolePage; + + await setTimezoneInProfile(page, MOSCOW_TIMEZONE); // UTC+3 + + const onCallScheduleName = generateRandomValue(); + await createOnCallSchedule(page, onCallScheduleName, userName, false); + + await clickButton({ page, buttonText: 'Add override' }); + + const overrideStartEl = page.getByTestId('override-start'); + await changeDatePickerTime(overrideStartEl, '02'); + await expect(overrideStartEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('02:00'); + + const overrideEndEl = page.getByTestId('override-end'); + await changeDatePickerTime(overrideEndEl, '12'); + await expect(overrideEndEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('12:00'); + + await page.getByRole('dialog').click(); // clear focus + + await page.getByTestId('timezone-select').locator('svg').click(); + await page.getByText('GMT', { exact: true }).click(); + + // expect times to go back by -3 + await expect(overrideStartEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('23:00'); + await expect(overrideEndEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('09:00'); + + async function changeDatePickerTime(element: Locator, value: string) { + await element.getByRole('img').click(); + // set minutes to {value} + await page.locator('.rc-time-picker-panel').getByRole('button', { name: value }).first().click(); + // set seconds to 00 + await page.getByRole('button', { name: '00' }).nth(1).click(); + } +}); diff --git a/grafana-plugin/e2e-tests/schedules/addRotation.test.ts b/grafana-plugin/e2e-tests/schedules/addRotation.test.ts new file mode 100644 index 00000000..478e6903 --- /dev/null +++ b/grafana-plugin/e2e-tests/schedules/addRotation.test.ts @@ -0,0 +1,43 @@ +import { test, expect, Locator } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; +import { clickButton, generateRandomValue } from '../utils/forms'; +import { setTimezoneInProfile } from '../utils/grafanaProfile'; +import { createOnCallSchedule } from '../utils/schedule'; + +test('Fills in Rotation time and reacts to timezone change', async ({ adminRolePage }) => { + const { page, userName } = adminRolePage; + + await setTimezoneInProfile(page, MOSCOW_TIMEZONE); // UTC+3 + + const onCallScheduleName = generateRandomValue(); + await createOnCallSchedule(page, onCallScheduleName, userName, false); + + await clickButton({ page, buttonText: 'Add rotation' }); + // enable Rotation End + await page.getByTestId('rotation-end').getByLabel('Toggle switch').click(); + + const startEl = page.getByTestId('rotation-start'); + await changeDatePickerTime(startEl, '02'); + await expect(startEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('02:00'); + + const endEl = page.getByTestId('rotation-end'); + await changeDatePickerTime(endEl, '12'); + await expect(endEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('12:00'); + + await page.getByRole('dialog').click(); // clear focus + + await page.getByTestId('timezone-select').locator('svg').click(); + await page.getByText('GMT', { exact: true }).click(); + + // expect times to go back by -3 + await expect(startEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('23:00'); + await expect(endEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('09:00'); + + async function changeDatePickerTime(element: Locator, value: string) { + await element.getByRole('img').click(); + // set minutes to {value} + await page.locator('.rc-time-picker-panel').getByRole('button', { name: value }).first().click(); + // set seconds to 00 + await page.getByRole('button', { name: '00' }).nth(1).click(); + } +}); diff --git a/grafana-plugin/e2e-tests/schedules/quality.test.ts b/grafana-plugin/e2e-tests/schedules/quality.test.ts index abd26d9c..82d84212 100644 --- a/grafana-plugin/e2e-tests/schedules/quality.test.ts +++ b/grafana-plugin/e2e-tests/schedules/quality.test.ts @@ -1,12 +1,12 @@ import { test, expect } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('check schedule quality for simple 1-user schedule', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); const scheduleQualityElement = page.getByTestId('schedule-quality'); await scheduleQualityElement.waitFor({ state: 'visible' }); diff --git a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts index e1bd7f32..e092bb61 100644 --- a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts +++ b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts @@ -1,13 +1,13 @@ import { test, expect } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation, createRotation } from '../utils/schedule'; +import { createOnCallSchedule, createRotation } from '../utils/schedule'; test(`user can see the other user's details`, async ({ adminRolePage, editorRolePage }) => { const { page, userName: adminUserName } = adminRolePage; const editorUserName = editorRolePage.userName; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, adminUserName); + await createOnCallSchedule(page, onCallScheduleName, adminUserName); await createRotation(page, editorUserName, false); await page.waitForTimeout(1_000); diff --git a/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts b/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts index 1def1645..0f27f47a 100644 --- a/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts +++ b/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts @@ -4,13 +4,13 @@ import { HTML_ID } from 'utils/DOM'; import { expect, test } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test.skip('schedule view (week/2 weeks/month) toggler works', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); // ScheduleView.OneWeek is selected by default expect(await page.getByLabel(ScheduleView.OneWeek, { exact: true }).isChecked()).toBe(true); diff --git a/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts b/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts index d83b570e..31c50c1f 100644 --- a/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts +++ b/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts @@ -1,13 +1,13 @@ import { expect, test } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; import { goToOnCallPage } from '../utils/navigation'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('schedule calendar and list of schedules is correctly displayed', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await goToOnCallPage(page, 'schedules'); diff --git a/grafana-plugin/e2e-tests/schedules/timezones.test.ts b/grafana-plugin/e2e-tests/schedules/timezones.test.ts index 32f5689d..71df96f6 100644 --- a/grafana-plugin/e2e-tests/schedules/timezones.test.ts +++ b/grafana-plugin/e2e-tests/schedules/timezones.test.ts @@ -4,15 +4,14 @@ import isoWeek from 'dayjs/plugin/isoWeek'; import utc from 'dayjs/plugin/utc'; import { test } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; import { clickButton, generateRandomValue } from '../utils/forms'; import { setTimezoneInProfile } from '../utils/grafanaProfile'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; dayjs.extend(utc); dayjs.extend(isoWeek); -const MOSCOW_TIMEZONE = 'Europe/Moscow'; - test.use({ timezoneId: MOSCOW_TIMEZONE }); // GMT+3 the whole year const currentUtcTimeHour = dayjs().utc().format('HH'); const currentUtcDate = dayjs().utc().format('DD MMM'); @@ -25,7 +24,7 @@ test('dates in schedule are correct according to selected current timezone', asy await setTimezoneInProfile(page, MOSCOW_TIMEZONE); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); // Current timezone is selected by default to currently logged in user timezone await expect(page.getByTestId('timezone-select')).toHaveText('GMT+3'); diff --git a/grafana-plugin/e2e-tests/utils/constants.ts b/grafana-plugin/e2e-tests/utils/constants.ts index f6969efd..5345d1c4 100644 --- a/grafana-plugin/e2e-tests/utils/constants.ts +++ b/grafana-plugin/e2e-tests/utils/constants.ts @@ -10,3 +10,5 @@ export const GRAFANA_ADMIN_PASSWORD = process.env.GRAFANA_ADMIN_PASSWORD || 'onc export const IS_OPEN_SOURCE = (process.env.IS_OPEN_SOURCE || 'true').toLowerCase() === 'true'; export const IS_CLOUD = !IS_OPEN_SOURCE; + +export const MOSCOW_TIMEZONE = 'Europe/Moscow'; diff --git a/grafana-plugin/e2e-tests/utils/grafanaProfile.ts b/grafana-plugin/e2e-tests/utils/grafanaProfile.ts index 672de8cd..add2e32f 100644 --- a/grafana-plugin/e2e-tests/utils/grafanaProfile.ts +++ b/grafana-plugin/e2e-tests/utils/grafanaProfile.ts @@ -1,11 +1,14 @@ -import { Page } from '@playwright/test'; +import { Page, expect } from '@playwright/test'; import { goToGrafanaPage } from './navigation'; export const setTimezoneInProfile = async (page: Page, timezone: string) => { await goToGrafanaPage(page, '/profile'); + await expect(page.getByLabel('Time zone picker')).toBeVisible(); + await page.getByLabel('Time zone picker').click(); await page.getByLabel('Select options menu').getByText(timezone).click(); await page.getByTestId('data-testid-shared-prefs-save').click(); await page.waitForLoadState('networkidle'); + await page.waitForTimeout(3000); // wait for reload }; diff --git a/grafana-plugin/e2e-tests/utils/schedule.ts b/grafana-plugin/e2e-tests/utils/schedule.ts index e0e64f44..c08172f1 100644 --- a/grafana-plugin/e2e-tests/utils/schedule.ts +++ b/grafana-plugin/e2e-tests/utils/schedule.ts @@ -4,10 +4,11 @@ import dayjs from 'dayjs'; import { clickButton, selectDropdownValue } from './forms'; import { goToOnCallPage } from './navigation'; -export const createOnCallScheduleWithRotation = async ( +export const createOnCallSchedule = async ( page: Page, scheduleName: string, - userName: string + userName: string, + withRotation = true ): Promise