From 6b8c1ac94e66757245ed92e8f5ec63c1caf738c9 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 31 Aug 2023 12:40:31 -0300 Subject: [PATCH 01/16] Truncate exported final shifts to match the requested period (#2924) When calculating on-call load, if an involved shift extends outside the requested time span you can get unexpected results. Truncate the returned events times to keep the details for the specified start/end dates. --- CHANGELOG.md | 1 + .../apps/public_api/tests/test_schedules.py | 44 ++++++++++++++++++- engine/apps/public_api/views/schedules.py | 7 +-- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7319ff5b..fb5b83dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Performance and UX tweaks to integrations page ([#2869](https://github.com/grafana/oncall/pull/2869)) - Expand users details in filter swaps internal endpoint ([#2921](https://github.com/grafana/oncall/pull/2921)) +- Truncate exported final shifts to match the requested period ([#2924](https://github.com/grafana/oncall/pull/2924)) ### Fixed diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 53903448..3834c8df 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -969,7 +969,7 @@ def test_oncall_shifts_export_from_ical_schedule( client = APIClient() url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key}) - response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-08-01", format="json", HTTP_AUTHORIZATION=token) + response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-07-31", format="json", HTTP_AUTHORIZATION=token) assert response.status_code == status.HTTP_200_OK expected_on_call_times = { @@ -1006,7 +1006,7 @@ def test_oncall_shifts_export_from_api_schedule( client = APIClient() url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key}) - response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-08-01", format="json", HTTP_AUTHORIZATION=token) + response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-07-31", format="json", HTTP_AUTHORIZATION=token) assert response.status_code == status.HTTP_200_OK expected_on_call_times = { @@ -1014,3 +1014,43 @@ def test_oncall_shifts_export_from_api_schedule( user2.public_primary_key: 30, # daily 2h * 15d } assert_expected_shifts_export_response(response, (user1, user2), expected_on_call_times) + + +@pytest.mark.django_db +def test_oncall_shifts_export_truncate_events( + make_organization_and_user_with_token, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, token = make_organization_and_user_with_token() + user1 = make_user(organization=organization) + + user1_public_primary_key = user1.public_primary_key + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + + # 24h shifts starting 9am on Mo, We and Fr + start_date = timezone.datetime(2023, 1, 1, 9, 0, 0, tzinfo=pytz.UTC) + make_on_call_shift( + organization=organization, + schedule=schedule, + shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, + frequency=CustomOnCallShift.FREQUENCY_DAILY, + priority_level=1, + interval=1, + by_day=["MO", "WE", "FR"], + start=start_date, + rolling_users=[{user1.pk: user1_public_primary_key}], + rotation_start=start_date, + duration=timezone.timedelta(hours=24), + ) + + client = APIClient() + + # request shifts on a Tu (ie. 00:00 - 09:00) + url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key}) + response = client.get(f"{url}?start_date=2023-01-03&end_date=2023-01-03", format="json", HTTP_AUTHORIZATION=token) + assert response.status_code == status.HTTP_200_OK + + expected_on_call_times = {user1_public_primary_key: 9} + assert_expected_shifts_export_response(response, (user1,), expected_on_call_times) diff --git a/engine/apps/public_api/views/schedules.py b/engine/apps/public_api/views/schedules.py index d8c5ec5c..88b1fe26 100644 --- a/engine/apps/public_api/views/schedules.py +++ b/engine/apps/public_api/views/schedules.py @@ -145,7 +145,7 @@ class OnCallScheduleChannelView(RateLimitHeadersMixin, UpdateSerializerMixin, Mo datetime_start = datetime.datetime.combine(start_date, datetime.time.min, tzinfo=pytz.UTC) datetime_end = datetime_start + datetime.timedelta( - days=days_between_start_and_end - 1, hours=23, minutes=59, seconds=59 + days=days_between_start_and_end, hours=23, minutes=59, seconds=59 ) final_schedule_events: ScheduleEvents = schedule.final_events(datetime_start, datetime_end) @@ -158,8 +158,9 @@ class OnCallScheduleChannelView(RateLimitHeadersMixin, UpdateSerializerMixin, Mo "user_pk": user["pk"], "user_email": user["email"], "user_username": user["display_name"], - "shift_start": event["start"], - "shift_end": event["end"], + # truncate shift start/end exceeding the requested period + "shift_start": event["start"] if event["start"] >= datetime_start else datetime_start, + "shift_end": event["end"] if event["end"] <= datetime_end else datetime_end, } for event in final_schedule_events for user in event["users"] From 56c0c023ad28553cf77b7f4587ffe2f6e1820642 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 31 Aug 2023 12:42:08 -0600 Subject: [PATCH 02/16] Test celery tasks have queue assignment (#2922) # What this PR does - Add a test to ensure celery tasks are assigned to a queue - Move CELERY_TASK_ROUTES out of settings into its own file for easier reuse and reference. ## Which issue(s) this PR fixes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../tests/test_task_queue_assignment.py | 35 ++++ engine/settings/celery_task_routes.py | 167 ++++++++++++++++++ engine/settings/prod_without_db.py | 114 +----------- 3 files changed, 204 insertions(+), 112 deletions(-) create mode 100644 engine/common/tests/test_task_queue_assignment.py create mode 100644 engine/settings/celery_task_routes.py diff --git a/engine/common/tests/test_task_queue_assignment.py b/engine/common/tests/test_task_queue_assignment.py new file mode 100644 index 00000000..1acb5a0a --- /dev/null +++ b/engine/common/tests/test_task_queue_assignment.py @@ -0,0 +1,35 @@ +from celery import current_app + +from settings.celery_task_routes import CELERY_TASK_ROUTES + +""" +If a task has a legitimate reason to not have a queue assignment it can +be added here (In development, in process of deprecation, etc.) if possible +we should avoid @shared_dedicated_queue_retry_task or @shared_task and +remove entirely if it is not needed. +""" +COMMON_IGNORED_TASKS = { + "apps.mobile_app.tasks.new_alert_group.notify_user_async", + "apps.alerts.tasks.create_contact_points_for_datasource.schedule_create_contact_points_for_datasource", + "common.oncall_gateway.tasks.create_slack_connector_async_v2", +} + + +def check_celery_task_route_mapping(task_ids, ignored_prefixes, additional_ignored_tasks=None): + tasks = set(k for k in current_app.tasks.keys() if not k.startswith(ignored_prefixes)) + tasks -= set(COMMON_IGNORED_TASKS) + if additional_ignored_tasks: + tasks -= set(additional_ignored_tasks) + tasks -= set(task_ids) + if tasks: + print(f"Unassigned queue for celery task {tasks}") + assert len(tasks) == 0 + + +def test_celery_task_route_mapping(): + """ + If this test does not pass make sure you have added any newly added + @shared_dedicated_queue_retry_task or @shared_task to CELERY_TASK_ROUTES + in engine/settings/celery_task_routes.py + """ + check_celery_task_route_mapping(CELERY_TASK_ROUTES.keys(), ("extensions", "celery")) diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py new file mode 100644 index 00000000..2e4f6e4d --- /dev/null +++ b/engine/settings/celery_task_routes.py @@ -0,0 +1,167 @@ +CELERY_TASK_ROUTES = { + # DEFAULT + "apps.alerts.tasks.create_contact_points_for_datasource.create_contact_points_for_datasource": {"queue": "default"}, + "apps.alerts.tasks.sync_grafana_alerting_contact_points.disconnect_integration_from_alerting_contact_points": { + "queue": "default" + }, + "apps.alerts.tasks.sync_grafana_alerting_contact_points.sync_grafana_alerting_contact_points": {"queue": "default"}, + "apps.alerts.tasks.delete_alert_group.delete_alert_group": {"queue": "default"}, + "apps.alerts.tasks.invalidate_web_cache_for_alert_group.invalidate_web_cache_for_alert_group": {"queue": "default"}, + "apps.alerts.tasks.send_alert_group_signal.send_alert_group_signal": {"queue": "default"}, + "apps.alerts.tasks.wipe.wipe": {"queue": "default"}, + "common.oncall_gateway.tasks.create_oncall_connector_async": {"queue": "default"}, + "common.oncall_gateway.tasks.delete_oncall_connector_async": {"queue": "default"}, + "common.oncall_gateway.tasks.create_slack_connector_async": {"queue": "default"}, + "common.oncall_gateway.tasks.delete_slack_connector_async": {"queue": "default"}, + "common.oncall_gateway.tasks.delete_slack_connector_async_v2": {"queue": "default"}, + "apps.heartbeat.tasks.integration_heartbeat_checkup": {"queue": "default"}, + "apps.heartbeat.tasks.process_heartbeat_task": {"queue": "default"}, + "apps.metrics_exporter.tasks.start_calculate_and_cache_metrics": {"queue": "default"}, + "apps.metrics_exporter.tasks.start_recalculation_for_new_metric": {"queue": "default"}, + "apps.metrics_exporter.tasks.save_organizations_ids_in_cache": {"queue": "default"}, + "apps.mobile_app.tasks.new_shift_swap_request.notify_shift_swap_requests": {"queue": "default"}, + "apps.mobile_app.tasks.new_shift_swap_request.notify_shift_swap_request": {"queue": "default"}, + "apps.mobile_app.tasks.new_shift_swap_request.notify_user_about_shift_swap_request": {"queue": "default"}, + "apps.schedules.tasks.refresh_ical_files.refresh_ical_file": {"queue": "default"}, + "apps.schedules.tasks.refresh_ical_files.start_refresh_ical_files": {"queue": "default"}, + "apps.schedules.tasks.refresh_ical_files.refresh_ical_final_schedule": {"queue": "default"}, + "apps.schedules.tasks.refresh_ical_files.start_refresh_ical_final_schedules": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.check_empty_shifts_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.start_notify_about_gaps_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.check_gaps_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.notify_about_empty_shifts_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.schedule_notify_about_gaps_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.start_check_empty_shifts_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.start_check_gaps_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.start_notify_about_empty_shifts_in_schedule": { + "queue": "default" + }, + "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.check_empty_shifts_in_schedule": {"queue": "default"}, + "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.notify_about_empty_shifts_in_schedule": { + "queue": "default" + }, + "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.start_check_empty_shifts_in_schedule": { + "queue": "default" + }, + "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.start_notify_about_empty_shifts_in_schedule": { + "queue": "default" + }, + "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.schedule_notify_about_empty_shifts_in_schedule": { + "queue": "default" + }, + "apps.schedules.tasks.shift_swaps.slack_messages.create_shift_swap_request_message": {"queue": "default"}, + "apps.schedules.tasks.shift_swaps.slack_messages.update_shift_swap_request_message": {"queue": "default"}, + "apps.schedules.tasks.shift_swaps.slack_followups.send_shift_swap_request_slack_followups": {"queue": "default"}, + "apps.schedules.tasks.shift_swaps.slack_followups.send_shift_swap_request_slack_followup": {"queue": "default"}, + "apps.migration_tool.tasks.start_migration_from_old_amixr": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_schedules": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_integrations": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_routes": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_escalation_policies": {"queue": "default"}, + "apps.migration_tool.tasks.start_migration_alert_groups": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_alert_group": {"queue": "default"}, + "apps.migration_tool.tasks.start_migration_alerts": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_alert": {"queue": "default"}, + "apps.migration_tool.tasks.start_migration_logs": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_log": {"queue": "default"}, + "apps.migration_tool.tasks.start_migration_user_data": {"queue": "default"}, + "apps.migration_tool.tasks.migrate_user_data": {"queue": "default"}, + "apps.schedules.tasks.notify_about_gaps_in_schedule.notify_about_gaps_in_schedule": {"queue": "default"}, + "celery.backend_cleanup": {"queue": "default"}, + "apps.heartbeat.tasks.check_heartbeats": {"queue": "default"}, + "apps.oss_installation.tasks.send_cloud_heartbeat_task": {"queue": "default"}, + "apps.oss_installation.tasks.send_usage_stats_report": {"queue": "default"}, + "apps.oss_installation.tasks.sync_users_with_cloud": {"queue": "default"}, + # CRITICAL + "apps.alerts.tasks.acknowledge_reminder.acknowledge_reminder_task": {"queue": "critical"}, + "apps.alerts.tasks.acknowledge_reminder.unacknowledge_timeout_task": {"queue": "critical"}, + "apps.alerts.tasks.distribute_alert.distribute_alert": {"queue": "critical"}, + "apps.alerts.tasks.distribute_alert.send_alert_create_signal": {"queue": "critical"}, + "apps.alerts.tasks.escalate_alert_group.escalate_alert_group": {"queue": "critical"}, + "apps.alerts.tasks.invite_user_to_join_incident.invite_user_to_join_incident": {"queue": "critical"}, + "apps.alerts.tasks.maintenance.check_maintenance_finished": {"queue": "critical"}, + "apps.alerts.tasks.maintenance.disable_maintenance": {"queue": "critical"}, + "apps.alerts.tasks.notify_all.notify_all_task": {"queue": "critical"}, + "apps.alerts.tasks.notify_group.notify_group_task": {"queue": "critical"}, + "apps.alerts.tasks.notify_ical_schedule_shift.notify_ical_schedule_shift": {"queue": "critical"}, + "apps.alerts.tasks.notify_user.notify_user_task": {"queue": "critical"}, + "apps.alerts.tasks.notify_user.perform_notification": {"queue": "critical"}, + "apps.alerts.tasks.notify_user.send_user_notification_signal": {"queue": "critical"}, + "apps.alerts.tasks.resolve_alert_group_by_source_if_needed.resolve_alert_group_by_source_if_needed": { + "queue": "critical" + }, + "apps.alerts.tasks.resolve_by_last_step.resolve_by_last_step_task": {"queue": "critical"}, + "apps.alerts.tasks.send_update_log_report_signal.send_update_log_report_signal": {"queue": "critical"}, + "apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal": {"queue": "critical"}, + "apps.alerts.tasks.unsilence.unsilence_task": {"queue": "critical"}, + "apps.base.tasks.process_failed_to_invoke_celery_tasks": {"queue": "critical"}, + "apps.base.tasks.process_failed_to_invoke_celery_tasks_batch": {"queue": "critical"}, + "apps.email.tasks.notify_user_async": {"queue": "critical"}, + "apps.integrations.tasks.create_alert": {"queue": "critical"}, + "apps.integrations.tasks.create_alertmanager_alerts": {"queue": "critical"}, + "apps.integrations.tasks.start_notify_about_integration_ratelimit": {"queue": "critical"}, + # TODO: remove apps.mobile_app.tasks.notify_user_async in a future release + "apps.mobile_app.tasks.notify_user_async": {"queue": "critical"}, + "apps.mobile_app.tasks.new_alert_group.notify_user_about_new_alert_group": {"queue": "critical"}, + "apps.mobile_app.tasks.going_oncall_notification.conditionally_send_going_oncall_push_notifications_for_schedule": { + "queue": "critical" + }, + "apps.mobile_app.tasks.going_oncall_notification.conditionally_send_going_oncall_push_notifications_for_all_schedules": { + "queue": "critical" + }, + "apps.schedules.tasks.drop_cached_ical.drop_cached_ical_for_custom_events_for_organization": {"queue": "critical"}, + "apps.schedules.tasks.drop_cached_ical.drop_cached_ical_task": {"queue": "critical"}, + # GRAFANA + "apps.grafana_plugin.tasks.sync.plugin_sync_organization_async": {"queue": "grafana"}, + # LONG + "apps.alerts.tasks.alert_group_web_title_cache.update_web_title_cache_for_alert_receive_channel": {"queue": "long"}, + "apps.alerts.tasks.alert_group_web_title_cache.update_web_title_cache": {"queue": "long"}, + "apps.alerts.tasks.check_escalation_finished.check_escalation_finished_task": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.cleanup_organization_async": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.start_cleanup_deleted_organizations": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.start_sync_organizations": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.sync_organization_async": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.sync_team_members_for_organization_async": {"queue": "long"}, + "apps.grafana_plugin.tasks.sync.start_sync_regions": {"queue": "long"}, + "apps.metrics_exporter.tasks.calculate_and_cache_metrics": {"queue": "long"}, + "apps.metrics_exporter.tasks.calculate_and_cache_user_was_notified_metric": {"queue": "long"}, + # SLACK + "apps.integrations.tasks.notify_about_integration_ratelimit_in_slack": {"queue": "slack"}, + "apps.slack.helpers.alert_group_representative.on_alert_group_action_triggered_async": {"queue": "slack"}, + "apps.slack.helpers.alert_group_representative.on_alert_group_update_log_report_async": {"queue": "slack"}, + "apps.slack.helpers.alert_group_representative.on_create_alert_slack_representative_async": {"queue": "slack"}, + "apps.slack.tasks.clean_slack_channel_leftovers": {"queue": "slack"}, + "apps.slack.tasks.check_slack_message_exists_before_post_message_to_thread": {"queue": "slack"}, + "apps.slack.tasks.clean_slack_integration_leftovers": {"queue": "slack"}, + "apps.slack.tasks.populate_slack_channels": {"queue": "slack"}, + "apps.slack.tasks.populate_slack_channels_for_team": {"queue": "slack"}, + "apps.slack.tasks.populate_slack_user_identities": {"queue": "slack"}, + "apps.slack.tasks.populate_slack_usergroups": {"queue": "slack"}, + "apps.slack.tasks.populate_slack_usergroups_for_team": {"queue": "slack"}, + "apps.slack.tasks.post_or_update_log_report_message_task": {"queue": "slack"}, + "apps.slack.tasks.post_slack_rate_limit_message": {"queue": "slack"}, + "apps.slack.tasks.send_message_to_thread_if_bot_not_in_channel": {"queue": "slack"}, + "apps.slack.tasks.start_update_slack_user_group_for_schedules": {"queue": "slack"}, + "apps.slack.tasks.unpopulate_slack_user_identities": {"queue": "slack"}, + "apps.slack.tasks.update_incident_slack_message": {"queue": "slack"}, + "apps.slack.tasks.update_slack_user_group_for_schedules": {"queue": "slack"}, + "apps.slack.representatives.alert_group_representative.on_create_alert_slack_representative_async": { + "queue": "slack" + }, + "apps.slack.representatives.alert_group_representative.on_alert_group_action_triggered_async": {"queue": "slack"}, + "apps.slack.representatives.alert_group_representative.on_alert_group_update_log_report_async": {"queue": "slack"}, + # TELEGRAM + "apps.telegram.tasks.edit_message": {"queue": "telegram"}, + "apps.telegram.tasks.on_create_alert_telegram_representative_async": {"queue": "telegram"}, + "apps.telegram.tasks.register_telegram_webhook": {"queue": "telegram"}, + "apps.telegram.tasks.send_link_to_channel_message_or_fallback_to_full_alert_group": {"queue": "telegram"}, + "apps.telegram.tasks.send_log_and_actions_message": {"queue": "telegram"}, + # WEBHOOK + "apps.alerts.tasks.custom_button_result.custom_button_result": {"queue": "webhook"}, + "apps.alerts.tasks.custom_webhook_result.custom_webhook_result": {"queue": "webhook"}, + "apps.mobile_app.fcm_relay.fcm_relay_async": {"queue": "webhook"}, + "apps.webhooks.tasks.trigger_webhook.execute_webhook": {"queue": "webhook"}, + "apps.webhooks.tasks.trigger_webhook.send_webhook_event": {"queue": "webhook"}, + "apps.webhooks.tasks.alert_group_status.alert_group_created": {"queue": "webhook"}, + "apps.webhooks.tasks.alert_group_status.alert_group_status_change": {"queue": "webhook"}, +} diff --git a/engine/settings/prod_without_db.py b/engine/settings/prod_without_db.py index c5bab5d2..9b4e7682 100644 --- a/engine/settings/prod_without_db.py +++ b/engine/settings/prod_without_db.py @@ -1,5 +1,6 @@ import os +from . import celery_task_routes from .base import * # noqa: F401, F403 try: @@ -44,118 +45,7 @@ SECURE_REDIRECT_EXEMPT = [ ] SECURE_HSTS_SECONDS = 360000 -CELERY_TASK_ROUTES = { - # DEFAULT - "apps.alerts.tasks.create_contact_points_for_datasource.create_contact_points_for_datasource": {"queue": "default"}, - "apps.alerts.tasks.sync_grafana_alerting_contact_points.sync_grafana_alerting_contact_points": {"queue": "default"}, - "apps.alerts.tasks.sync_grafana_alerting_contact_points.disconnect_integration_from_alerting_contact_points": { - "queue": "default" - }, - "apps.alerts.tasks.delete_alert_group.delete_alert_group": {"queue": "default"}, - "apps.alerts.tasks.send_alert_group_signal.send_alert_group_signal": {"queue": "default"}, - "apps.alerts.tasks.wipe.wipe": {"queue": "default"}, - "apps.heartbeat.tasks.integration_heartbeat_checkup": {"queue": "default"}, - "apps.heartbeat.tasks.process_heartbeat_task": {"queue": "default"}, - "apps.metrics_exporter.tasks.start_calculate_and_cache_metrics": {"queue": "default"}, - "apps.metrics_exporter.tasks.start_recalculation_for_new_metric": {"queue": "default"}, - "apps.metrics_exporter.tasks.save_organizations_ids_in_cache": {"queue": "default"}, - "apps.mobile_app.tasks.notify_shift_swap_requests": {"queue": "default"}, - "apps.mobile_app.tasks.notify_shift_swap_request": {"queue": "default"}, - "apps.mobile_app.tasks.notify_user_about_shift_swap_request": {"queue": "default"}, - "apps.schedules.tasks.refresh_ical_files.refresh_ical_file": {"queue": "default"}, - "apps.schedules.tasks.refresh_ical_files.start_refresh_ical_files": {"queue": "default"}, - "apps.schedules.tasks.notify_about_gaps_in_schedule.check_empty_shifts_in_schedule": {"queue": "default"}, - "apps.schedules.tasks.notify_about_gaps_in_schedule.notify_about_empty_shifts_in_schedule": {"queue": "default"}, - "apps.schedules.tasks.notify_about_gaps_in_schedule.start_check_empty_shifts_in_schedule": {"queue": "default"}, - "apps.schedules.tasks.notify_about_gaps_in_schedule.start_notify_about_empty_shifts_in_schedule": { - "queue": "default" - }, - "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.check_empty_shifts_in_schedule": {"queue": "default"}, - "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.notify_about_empty_shifts_in_schedule": { - "queue": "default" - }, - "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.start_check_empty_shifts_in_schedule": { - "queue": "default" - }, - "apps.schedules.tasks.notify_about_empty_shifts_in_schedule.start_notify_about_empty_shifts_in_schedule": { - "queue": "default" - }, - "apps.schedules.tasks.shift_swaps.slack_messages.create_shift_swap_request_message": {"queue": "default"}, - "apps.schedules.tasks.shift_swaps.slack_messages.update_shift_swap_request_message": {"queue": "default"}, - "apps.schedules.tasks.shift_swaps.slack_followups.send_shift_swap_request_slack_followups": {"queue": "default"}, - "apps.schedules.tasks.shift_swaps.slack_followups.send_shift_swap_request_slack_followup": {"queue": "default"}, - # CRITICAL - "apps.alerts.tasks.acknowledge_reminder.acknowledge_reminder_task": {"queue": "critical"}, - "apps.alerts.tasks.acknowledge_reminder.unacknowledge_timeout_task": {"queue": "critical"}, - "apps.alerts.tasks.distribute_alert.distribute_alert": {"queue": "critical"}, - "apps.alerts.tasks.distribute_alert.send_alert_create_signal": {"queue": "critical"}, - "apps.alerts.tasks.escalate_alert_group.escalate_alert_group": {"queue": "critical"}, - "apps.alerts.tasks.invite_user_to_join_incident.invite_user_to_join_incident": {"queue": "critical"}, - "apps.alerts.tasks.maintenance.check_maintenance_finished": {"queue": "critical"}, - "apps.alerts.tasks.maintenance.disable_maintenance": {"queue": "critical"}, - "apps.alerts.tasks.notify_all.notify_all_task": {"queue": "critical"}, - "apps.alerts.tasks.notify_group.notify_group_task": {"queue": "critical"}, - "apps.alerts.tasks.notify_ical_schedule_shift.notify_ical_schedule_shift": {"queue": "critical"}, - "apps.alerts.tasks.notify_user.notify_user_task": {"queue": "critical"}, - "apps.alerts.tasks.notify_user.perform_notification": {"queue": "critical"}, - "apps.alerts.tasks.notify_user.send_user_notification_signal": {"queue": "critical"}, - "apps.alerts.tasks.resolve_alert_group_by_source_if_needed.resolve_alert_group_by_source_if_needed": { - "queue": "critical" - }, - "apps.alerts.tasks.resolve_by_last_step.resolve_by_last_step_task": {"queue": "critical"}, - "apps.alerts.tasks.send_update_log_report_signal.send_update_log_report_signal": {"queue": "critical"}, - "apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal": {"queue": "critical"}, - "apps.alerts.tasks.unsilence.unsilence_task": {"queue": "critical"}, - "apps.base.tasks.process_failed_to_invoke_celery_tasks": {"queue": "critical"}, - "apps.base.tasks.process_failed_to_invoke_celery_tasks_batch": {"queue": "critical"}, - "apps.email.tasks.notify_user_async": {"queue": "critical"}, - "apps.integrations.tasks.create_alert": {"queue": "critical"}, - "apps.integrations.tasks.create_alertmanager_alerts": {"queue": "critical"}, - "apps.integrations.tasks.start_notify_about_integration_ratelimit": {"queue": "critical"}, - # TODO: remove apps.mobile_app.tasks.notify_user_async in a future release - "apps.mobile_app.tasks.notify_user_async": {"queue": "critical"}, - "apps.mobile_app.tasks.notify_user_about_new_alert_group": {"queue": "critical"}, - "apps.schedules.tasks.drop_cached_ical.drop_cached_ical_for_custom_events_for_organization": {"queue": "critical"}, - "apps.schedules.tasks.drop_cached_ical.drop_cached_ical_task": {"queue": "critical"}, - # LONG - "apps.alerts.tasks.alert_group_web_title_cache.update_web_title_cache_for_alert_receive_channel": {"queue": "long"}, - "apps.alerts.tasks.alert_group_web_title_cache.update_web_title_cache": {"queue": "long"}, - "apps.alerts.tasks.check_escalation_finished.check_escalation_finished_task": {"queue": "long"}, - "apps.grafana_plugin.tasks.sync.cleanup_organization_async": {"queue": "long"}, - "apps.grafana_plugin.tasks.sync.start_cleanup_deleted_organizations": {"queue": "long"}, - "apps.grafana_plugin.tasks.sync.start_sync_organizations": {"queue": "long"}, - "apps.grafana_plugin.tasks.sync.sync_organization_async": {"queue": "long"}, - "apps.metrics_exporter.tasks.calculate_and_cache_metrics": {"queue": "long"}, - "apps.metrics_exporter.tasks.calculate_and_cache_user_was_notified_metric": {"queue": "long"}, - # SLACK - "apps.integrations.tasks.notify_about_integration_ratelimit_in_slack": {"queue": "slack"}, - "apps.slack.helpers.alert_group_representative.on_alert_group_action_triggered_async": {"queue": "slack"}, - "apps.slack.helpers.alert_group_representative.on_alert_group_update_log_report_async": {"queue": "slack"}, - "apps.slack.helpers.alert_group_representative.on_create_alert_slack_representative_async": {"queue": "slack"}, - "apps.slack.tasks.check_slack_message_exists_before_post_message_to_thread": {"queue": "slack"}, - "apps.slack.tasks.clean_slack_integration_leftovers": {"queue": "slack"}, - "apps.slack.tasks.populate_slack_channels": {"queue": "slack"}, - "apps.slack.tasks.populate_slack_channels_for_team": {"queue": "slack"}, - "apps.slack.tasks.populate_slack_user_identities": {"queue": "slack"}, - "apps.slack.tasks.populate_slack_usergroups": {"queue": "slack"}, - "apps.slack.tasks.populate_slack_usergroups_for_team": {"queue": "slack"}, - "apps.slack.tasks.post_or_update_log_report_message_task": {"queue": "slack"}, - "apps.slack.tasks.post_slack_rate_limit_message": {"queue": "slack"}, - "apps.slack.tasks.send_message_to_thread_if_bot_not_in_channel": {"queue": "slack"}, - "apps.slack.tasks.start_update_slack_user_group_for_schedules": {"queue": "slack"}, - "apps.slack.tasks.unpopulate_slack_user_identities": {"queue": "slack"}, - "apps.slack.tasks.update_incident_slack_message": {"queue": "slack"}, - "apps.slack.tasks.update_slack_user_group_for_schedules": {"queue": "slack"}, - # TELEGRAM - "apps.telegram.tasks.edit_message": {"queue": "telegram"}, - "apps.telegram.tasks.on_create_alert_telegram_representative_async": {"queue": "telegram"}, - "apps.telegram.tasks.register_telegram_webhook": {"queue": "telegram"}, - "apps.telegram.tasks.send_link_to_channel_message_or_fallback_to_full_alert_group": {"queue": "telegram"}, - "apps.telegram.tasks.send_log_and_actions_message": {"queue": "telegram"}, - # WEBHOOK - "apps.alerts.tasks.custom_button_result.custom_button_result": {"queue": "webhook"}, - "apps.mobile_app.fcm_relay.fcm_relay_async": {"queue": "webhook"}, -} +CELERY_TASK_ROUTES = celery_task_routes.CELERY_TASK_ROUTES REST_FRAMEWORK = { "DEFAULT_PARSER_CLASSES": ( From 15e02c7f3049b74bda50935d77ed2b6c5198f230 Mon Sep 17 00:00:00 2001 From: Simon Crute Date: Thu, 31 Aug 2023 20:15:27 +0100 Subject: [PATCH 03/16] Remove duplicate line (#2934) remove duplicate line in first example. # What this PR does ## Which issue(s) this PR fixes ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- docs/sources/oncall-api-reference/users.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/sources/oncall-api-reference/users.md b/docs/sources/oncall-api-reference/users.md index f703e870..b3e58821 100644 --- a/docs/sources/oncall-api-reference/users.md +++ b/docs/sources/oncall-api-reference/users.md @@ -8,7 +8,6 @@ weight: 1500 This endpoint retrieves the user object. -````shell ```shell curl "{{API_URL}}/api/v1/users/current/" \ --request GET \ From 73da83274abf95f7581f81d93879f516b80d9850 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Sep 2023 07:53:27 +0200 Subject: [PATCH 04/16] Update Shift Swap Request Slack message formatting (#2918) # What this PR does Updates Shift Swap Request Slack message formatting to be consistent with UI mockups: **New Request** ![Screenshot 2023-08-30 at 12 18 54](https://github.com/grafana/oncall/assets/9406895/712a13e2-b768-4be6-b066-c5daa98446eb) **Accepted** ![Screenshot 2023-08-30 at 12 19 41](https://github.com/grafana/oncall/assets/9406895/84d14adf-bb48-4ba8-bee2-eb7931ef2d55) **Deleted** ![Screenshot 2023-08-30 at 12 19 01](https://github.com/grafana/oncall/assets/9406895/9bda06c8-5ce7-405b-9303-b160eac4d635) ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 1 + .../on-call-schedules/shift-swaps/_index.md | 10 +-- .../slack/scenarios/shift_swap_requests.py | 70 +++++++++---------- .../test_shift_swap_requests.py | 30 +++----- 4 files changed, 48 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb5b83dc..fb1e39cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Update Shift Swap Request Slack message formatting by @joeyorlando ([#2918](https://github.com/grafana/oncall/pull/2918)) - Performance and UX tweaks to integrations page ([#2869](https://github.com/grafana/oncall/pull/2869)) - Expand users details in filter swaps internal endpoint ([#2921](https://github.com/grafana/oncall/pull/2921)) - Truncate exported final shifts to match the requested period ([#2924](https://github.com/grafana/oncall/pull/2924)) diff --git a/docs/sources/on-call-schedules/shift-swaps/_index.md b/docs/sources/on-call-schedules/shift-swaps/_index.md index 97858cbf..03340a09 100644 --- a/docs/sources/on-call-schedules/shift-swaps/_index.md +++ b/docs/sources/on-call-schedules/shift-swaps/_index.md @@ -36,14 +36,14 @@ of a schedule, or clicking the button shown when hovering on a particular shift ->**Note**: no recurrence rules support is available when requesting a shift swap. If you need to recurrently change a shift, -consider creating a higher level layer rotation with the desired updates. +> **Note**: no recurrence rules support is available when requesting a shift swap. If you need to recurrently change a shift, +> consider creating a higher level layer rotation with the desired updates. Upon submitting the request, a Slack notification will be sent to the channel associated to the correspondent schedule, if there is one. A [mobile push notification][shift-swap-notifications] will be sent to team members who participate in the schedule and have the notifications enabled. - + Push notifications are sent 4 weeks ahead of the requested shift swap, or shortly after creation in case the shift swap start time is less than 4 weeks away, but always during users' working hours (by default 9am-5pm on @@ -64,8 +64,8 @@ The follow-up notifications will be sent at the following intervals before the s You can delete the swap request at any time. If the swap has been taken, it will automatically be undone upon removal. ->**Note**: if [RBAC][rbac] is enabled, a user is required to have the `SCHEDULES_WRITE` permission to create, -update, take or delete a swap request. `SCHEDULES_READ` will be enough to get details about existing requests. +> **Note**: if [RBAC][rbac] is enabled, a user is required to have the `SCHEDULES_WRITE` permission to create, +> update, take or delete a swap request. `SCHEDULES_READ` will be enough to get details about existing requests. ## Check existing swap requests diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 525ab730..18711b55 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -5,7 +5,6 @@ import typing import humanize from django.utils import timezone -from apps.slack.constants import DIVIDER from apps.slack.models import SlackMessage from apps.slack.scenarios import scenario_step from apps.slack.types import Block, BlockActionType, EventPayload, PayloadType, ScenarioRoute @@ -21,17 +20,26 @@ logger.setLevel(logging.DEBUG) SHIFT_SWAP_PK_ACTION_KEY = "shift_swap_request_pk" +def _schedule_slack_url(shift_swap_request) -> str: + schedule = shift_swap_request.schedule + return f"<{schedule.web_detail_page_link}|{schedule.name}>" + + class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyBlocks: pk = shift_swap_request.pk - main_message_text = f"Your teammate {shift_swap_request.beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + main_message_text = ( + f"*New shift swap request for {_schedule_slack_url(shift_swap_request)}*\n" + f"Your teammate {shift_swap_request.beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + ) datetime_format = SlackDateFormat.DATE_LONG_PRETTY time_format = SlackDateFormat.TIME shift_details = "" - for shift in shift_swap_request.shifts(): + shifts = shift_swap_request.shifts() + for shift in shifts: shift_start = shift["start"] shift_start_posix = shift_start.timestamp() shift_end = shift["end"] @@ -58,18 +66,22 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): }, }, ), - typing.cast( - Block.Section, - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": f"*📅 Shift Details*:\n\n{shift_details}", - }, - }, - ), ] + if shifts: + blocks.append( + typing.cast( + Block.Section, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": f"*Shift detail{'s' if len(shifts) > 1 else ''}*\n{shift_details}", + }, + }, + ), + ) + if description := shift_swap_request.description: blocks.append( typing.cast( @@ -78,7 +90,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): "type": "section", "text": { "type": "mrkdwn", - "text": f"*📝 Description*: {description}", + "text": f"*Description*\n{description}", }, }, ) @@ -92,7 +104,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): "type": "section", "text": { "type": "mrkdwn", - "text": "*Update*: this shift swap request has been deleted.", + "text": "❌ this shift swap request has been deleted", }, }, ), @@ -105,7 +117,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): "type": "section", "text": { "type": "mrkdwn", - "text": f"*Update*: {shift_swap_request.benefactor.get_username_with_slack_verbal()} has taken the shift swap.", + "text": f"✅ {shift_swap_request.benefactor.get_username_with_slack_verbal()} has accepted the shift swap request", }, }, ), @@ -124,10 +136,9 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): "elements": [ { "type": "button", - "style": "primary", "text": { "type": "plain_text", - "text": "✔️ Accept Shift Swap Request", + "text": "Accept", "emoji": True, }, "value": json.dumps(value), @@ -138,24 +149,6 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): ) ) - blocks.extend( - [ - DIVIDER, - typing.cast( - Block.Context, - { - "type": "context", - "elements": [ - { - "type": "mrkdwn", - "text": f"👀 View the shift swap within Grafana OnCall by clicking <{shift_swap_request.web_link}|here>.", - }, - ], - }, - ), - ] - ) - return blocks def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage: @@ -226,8 +219,9 @@ class ShiftSwapRequestFollowUp(scenario_step.ScenarioStep): "text": { "type": "mrkdwn", "text": ( - f":exclamation: This shift swap request is still open and will start in {delta}.\n" - "Jump back into the thread and accept it if you're available!" + f"⚠️ This shift swap request for {_schedule_slack_url(shift_swap_request)} is " + f"still open and will start in {delta}. Jump back into the thread and accept it if " + "you're available!" ), }, }, diff --git a/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py b/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py index a2089fbb..f480b6b4 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py @@ -40,26 +40,16 @@ class TestBaseShiftSwapRequestStep: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert ( - blocks[0]["text"]["text"] - == f"Your teammate {beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + assert blocks[0]["text"]["text"] == ( + f"*New shift swap request for <{ssr.schedule.web_detail_page_link}|{ssr.schedule.name}>*\n" + f"Your teammate {beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." ) - accept_button = blocks[2] + accept_button = blocks[1] - assert accept_button["elements"][0]["text"]["text"] == "✔️ Accept Shift Swap Request" + assert accept_button["elements"][0]["text"]["text"] == "Accept" assert accept_button["type"] == "actions" - assert blocks[3]["type"] == "divider" - - context_section = blocks[4] - - assert context_section["type"] == "context" - assert ( - context_section["elements"][0]["text"] - == f"👀 View the shift swap within Grafana OnCall by clicking <{ssr.web_link}|here>." - ) - @patch("apps.schedules.models.ShiftSwapRequest.shifts") @pytest.mark.parametrize( "shifts,expected_text", @@ -108,7 +98,7 @@ class TestBaseShiftSwapRequestStep: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[1]["text"]["text"] == f"*📅 Shift Details*:\n\n{expected_text}" + assert blocks[1]["text"]["text"] == f"*Shift details*\n{expected_text}" @pytest.mark.django_db def test_generate_blocks_ssr_has_description(self, setup) -> None: @@ -118,7 +108,7 @@ class TestBaseShiftSwapRequestStep: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[2]["text"]["text"] == f"*📝 Description*: {description}" + assert blocks[1]["text"]["text"] == f"*Description*\n{description}" @pytest.mark.django_db def test_generate_blocks_ssr_is_deleted(self, setup) -> None: @@ -128,7 +118,7 @@ class TestBaseShiftSwapRequestStep: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[2]["text"]["text"] == "*Update*: this shift swap request has been deleted." + assert blocks[1]["text"]["text"] == "❌ this shift swap request has been deleted" @pytest.mark.django_db def test_generate_blocks_ssr_is_taken(self, setup) -> None: @@ -140,8 +130,8 @@ class TestBaseShiftSwapRequestStep: blocks = step._generate_blocks(ssr) assert ( - blocks[2]["text"]["text"] - == f"*Update*: {benefactor.get_username_with_slack_verbal()} has taken the shift swap." + blocks[1]["text"]["text"] + == f"✅ {benefactor.get_username_with_slack_verbal()} has accepted the shift swap request" ) @patch("apps.slack.scenarios.shift_swap_requests.BaseShiftSwapRequestStep._generate_blocks") From 1d089aa7a33bcd4317e67ee2778920a31838ea24 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Sep 2023 10:14:59 +0200 Subject: [PATCH 05/16] fix IntegrityErrors occuring when creating ResolutionNoteSlackMessage objects (#2933) # What this PR does ## Which issue(s) this PR fixes Closes https://github.com/grafana/oncall-private/issues/1822 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 +- .../migrations/0031_auto_20230831_1445.py | 21 + engine/apps/alerts/models/resolution_note.py | 5 + .../scenarios/slack_channel_integration.py | 61 +-- .../test_slack_channel_integration.py | 439 ++++++++++++++++++ 5 files changed, 491 insertions(+), 39 deletions(-) create mode 100644 engine/apps/alerts/migrations/0031_auto_20230831_1445.py create mode 100644 engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fb1e39cb..b929ab74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,10 +23,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix issue with helm chart when specifying `broker.type=rabbitmq` where Redis environment variables - were not longer being injected @joeyorlando ([#2927](https://github.com/grafana/oncall/pull/2927)) + were not longer being injected by @joeyorlando ([#2927](https://github.com/grafana/oncall/pull/2927)) - Fix silence for alert groups with empty escalation chain @Ferril ([#2929](https://github.com/grafana/oncall/pull/2929)) - Fixed NPE when migrating legacy Grafana Alerting integrations ([#2908](https://github.com/grafana/oncall/issues/2908)) +- Fix `IntegrityError` exceptions that occasionally would occur when trying to create `ResolutionNoteSlackMessage` + objects by @joeyorlando ([#2933](https://github.com/grafana/oncall/pull/2933)) ## v1.3.29 (2023-08-29) diff --git a/engine/apps/alerts/migrations/0031_auto_20230831_1445.py b/engine/apps/alerts/migrations/0031_auto_20230831_1445.py new file mode 100644 index 00000000..ea6cdd80 --- /dev/null +++ b/engine/apps/alerts/migrations/0031_auto_20230831_1445.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.20 on 2023-08-31 14:45 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0030_auto_20230731_0341'), + ] + + operations = [ + migrations.AddIndex( + model_name='resolutionnoteslackmessage', + index=models.Index(fields=['ts', 'thread_ts', 'alert_group_id'], name='alerts_reso_ts_08f72c_idx'), + ), + migrations.AddIndex( + model_name='resolutionnoteslackmessage', + index=models.Index(fields=['ts', 'thread_ts', 'slack_channel_id'], name='alerts_reso_ts_a9bdf7_idx'), + ), + ] diff --git a/engine/apps/alerts/models/resolution_note.py b/engine/apps/alerts/models/resolution_note.py index da65ab94..af1317c9 100644 --- a/engine/apps/alerts/models/resolution_note.py +++ b/engine/apps/alerts/models/resolution_note.py @@ -83,6 +83,11 @@ class ResolutionNoteSlackMessage(models.Model): class Meta: unique_together = ("thread_ts", "ts") + indexes = [ + models.Index(fields=["ts", "thread_ts", "alert_group_id"]), + models.Index(fields=["ts", "thread_ts", "slack_channel_id"]), + ] + def get_resolution_note(self) -> typing.Optional["ResolutionNote"]: try: return self.resolution_note diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index 8e1cb787..7a2eea9c 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -82,48 +82,33 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): if result["permalink"] is not None: permalink = result["permalink"] - try: - slack_thread_message = ResolutionNoteSlackMessage.objects.get( - ts=message_ts, - thread_ts=thread_ts, - alert_group=alert_group, + if len(text) > 2900: + self._slack_client.api_call( + "chat.postEphemeral", + channel=channel, + user=slack_user_identity.slack_id, + text=":warning: Unable to show the <{}|message> in Resolution Note: the message is too long ({}). " + "Max length - 2900 symbols.".format(permalink, len(text)), ) - if len(text) > 2900: - if slack_thread_message.added_to_resolution_note: - self._slack_client.api_call( - "chat.postEphemeral", - channel=channel, - user=slack_user_identity.slack_id, - text=":warning: Unable to update the <{}|message> in Resolution Note: the message is too long ({}). " - "Max length - 2900 symbols.".format(permalink, len(text)), - ) - return + return + + slack_thread_message, created = ResolutionNoteSlackMessage.objects.get_or_create( + ts=message_ts, + thread_ts=thread_ts, + alert_group=alert_group, + defaults={ + "user": self.user, + "added_by_user": self.user, + "text": text, + "slack_channel_id": channel, + "permalink": permalink, + }, + ) + + if not created: slack_thread_message.text = text slack_thread_message.save() - except ResolutionNoteSlackMessage.DoesNotExist: - if len(text) > 2900: - self._slack_client.api_call( - "chat.postEphemeral", - channel=channel, - user=slack_user_identity.slack_id, - text=":warning: The <{}|message> will not be displayed in Resolution Note: " - "the message is too long ({}). Max length - 2900 symbols.".format(permalink, len(text)), - ) - return - - slack_thread_message = ResolutionNoteSlackMessage( - alert_group=alert_group, - user=self.user, - added_by_user=self.user, - text=text, - slack_channel_id=channel, - thread_ts=thread_ts, - ts=message_ts, - permalink=permalink, - ) - slack_thread_message.save() - def delete_thread_message_from_resolution_note( self, slack_user_identity: "SlackUserIdentity", payload: EventPayload ) -> None: diff --git a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py new file mode 100644 index 00000000..6cc01525 --- /dev/null +++ b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py @@ -0,0 +1,439 @@ +from unittest.mock import Mock, call, patch + +import pytest + +from apps.alerts.models import ResolutionNoteSlackMessage +from apps.slack.scenarios.slack_channel_integration import SlackChannelMessageEventStep + + +@pytest.mark.django_db +class TestSlackChannelMessageEventStep: + @patch.object(SlackChannelMessageEventStep, "save_thread_message_for_resolution_note") + @patch.object(SlackChannelMessageEventStep, "delete_thread_message_from_resolution_note") + @pytest.mark.parametrize( + "payload,save_called,delete_called", + [ + ( + { + # does not have thread_ts attribute or subtype + "event": {}, + }, + False, + False, + ), + ( + { + # has thread_ts attribute but has subtype attribute that is not MESSAGE_CHANGED + "event": { + "thread_ts": "foo", + "subtype": "bar", + }, + }, + False, + False, + ), + ( + { + # has thread_ts attribute but not subtype attribute + "event": { + "thread_ts": "foo", + }, + }, + True, + False, + ), + # MESSAGE_CHANGED event.subtype scenarios + ( + { + "event": { + "subtype": "message_changed", + "message": { + "subtype": "bar", + "thread_ts": "hello", + }, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "message_changed", + "message": { + "subtype": "bar", + }, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "potato", + "message": { + "thread_ts": "bar", + }, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "message_changed", + "message": { + "thread_ts": "bar", + }, + }, + }, + True, + False, + ), + ( + { + "event": { + "subtype": "message_deleted", + "previous_message": {}, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "message_deleted", + "previous_message": {}, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "potato", + "previous_message": { + "thread_ts": "bar", + }, + }, + }, + False, + False, + ), + ( + { + "event": { + "subtype": "message_deleted", + "previous_message": { + "thread_ts": "bar", + }, + }, + }, + False, + True, + ), + ], + ) + def test_process_scenario( + self, + mock_delete_thread_message_from_resolution_note, + mock_save_thread_message_for_resolution_note, + make_organization_and_user_with_slack_identities, + payload, + save_called, + delete_called, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + if save_called: + mock_save_thread_message_for_resolution_note.assert_called_once_with(slack_user_identity, payload) + else: + mock_save_thread_message_for_resolution_note.assert_not_called() + + if delete_called: + mock_delete_thread_message_from_resolution_note.assert_called_once_with(slack_user_identity, payload) + else: + mock_delete_thread_message_from_resolution_note.assert_not_called() + + @patch("apps.alerts.models.ResolutionNoteSlackMessage") + def test_save_thread_message_for_resolution_note_no_slack_user_identity( + self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities + ) -> None: + organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step._slack_client = Mock() + + step.save_thread_message_for_resolution_note(None, {}) + + step._slack_client.api_call.assert_not_called() + MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called() + + @patch("apps.alerts.models.ResolutionNoteSlackMessage") + def test_save_thread_message_for_resolution_note_no_slack_message( + self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step._slack_client = Mock() + + payload = { + "event": { + "channel": "potato", + "ts": 88945.4849, + "thread_ts": 16789.123, + "text": "hello", + }, + } + + step.save_thread_message_for_resolution_note(slack_user_identity, payload) + + step._slack_client.api_call.assert_not_called() + MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called() + + @patch("apps.alerts.models.ResolutionNoteSlackMessage") + def test_save_thread_message_for_resolution_note_really_long_text( + self, + MockResolutionNoteSlackMessage, + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_message, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + integration = make_alert_receive_channel(organization) + alert_group = make_alert_group(integration) + + channel = "potato" + ts = 88945.4849 + thread_ts = 16789.123 + + make_slack_message(alert_group, slack_id=thread_ts, channel_id=channel) + + mock_permalink = "http://example.com" + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step._slack_client = Mock() + step._slack_client.api_call.side_effect = [{"permalink": mock_permalink}, None] + + payload = { + "event": { + "channel": channel, + "ts": ts, + "thread_ts": thread_ts, + "text": "h" * 2901, + }, + } + + step.save_thread_message_for_resolution_note(slack_user_identity, payload) + + step._slack_client.api_call.assert_has_calls( + [ + call( + "chat.getPermalink", + channel=payload["event"]["channel"], + message_ts=payload["event"]["ts"], + ), + call( + "chat.postEphemeral", + channel=payload["event"]["channel"], + user=slack_user_identity.slack_id, + text=":warning: Unable to show the <{}|message> in Resolution Note: the message is too long ({}). " + "Max length - 2900 symbols.".format(mock_permalink, len(payload["event"]["text"])), + ), + ] + ) + MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called() + + @pytest.mark.parametrize("resolution_note_slack_message_already_exists", [True, False]) + def test_save_thread_message_for_resolution_note( + self, + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_message, + make_resolution_note_slack_message, + resolution_note_slack_message_already_exists, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + integration = make_alert_receive_channel(organization) + alert_group = make_alert_group(integration) + + original_text = "original text" + new_text = "new text" + + channel = "potato" + ts = 88945.4849 + thread_ts = 16789.123 + + make_slack_message(alert_group, slack_id=thread_ts, channel_id=channel) + + resolution_note_slack_message = None + if resolution_note_slack_message_already_exists: + resolution_note_slack_message = make_resolution_note_slack_message( + alert_group, user, user, ts=ts, thread_ts=thread_ts, text=original_text + ) + + mock_permalink = "http://example.com" + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step._slack_client = Mock() + step._slack_client.api_call.side_effect = [{"permalink": mock_permalink}, None] + + payload = { + "event": { + "channel": channel, + "ts": ts, + "thread_ts": thread_ts, + "text": new_text, + }, + } + + step.save_thread_message_for_resolution_note(slack_user_identity, payload) + + step._slack_client.api_call.assert_has_calls( + [ + call( + "chat.getPermalink", + channel=payload["event"]["channel"], + message_ts=payload["event"]["ts"], + ), + ] + ) + + if resolution_note_slack_message_already_exists: + resolution_note_slack_message.refresh_from_db() + resolution_note_slack_message.text = new_text + else: + assert ( + ResolutionNoteSlackMessage.objects.filter( + ts=ts, + thread_ts=thread_ts, + alert_group=alert_group, + ).count() + == 1 + ) + + @patch("apps.alerts.models.ResolutionNoteSlackMessage") + def test_delete_thread_message_from_resolution_note_no_slack_user_identity( + self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step.delete_thread_message_from_resolution_note(None, {}) + + MockResolutionNoteSlackMessage.objects.get.assert_not_called() + + def test_delete_thread_message_from_resolution_note_no_message_found( + self, make_organization_and_user_with_slack_identities + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + + channel = "potato" + ts = 88945.4849 + thread_ts = 16789.123 + + payload = { + "event": { + "channel": channel, + "previous_message": { + "ts": ts, + "thread_ts": thread_ts, + }, + }, + } + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step.alert_group_slack_service = Mock() + + step.delete_thread_message_from_resolution_note(slack_user_identity, payload) + + step.alert_group_slack_service.assert_not_called() + + def test_delete_thread_message_from_resolution_note( + self, + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_resolution_note_slack_message, + ) -> None: + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + integration = make_alert_receive_channel(organization) + alert_group = make_alert_group(integration) + + channel = "potato" + ts = 88945.4849 + thread_ts = 16789.123 + + payload = { + "event": { + "channel": channel, + "previous_message": { + "ts": ts, + "thread_ts": thread_ts, + }, + }, + } + + make_resolution_note_slack_message( + alert_group, user, user, ts=ts, thread_ts=thread_ts, slack_channel_id=channel + ) + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step.alert_group_slack_service = Mock() + + step.delete_thread_message_from_resolution_note(slack_user_identity, payload) + + step.alert_group_slack_service.update_alert_group_slack_message.assert_called_once_with(alert_group) + assert ( + ResolutionNoteSlackMessage.objects.filter( + ts=ts, + thread_ts=thread_ts, + slack_channel_id=channel, + ).count() + == 0 + ) From 585bbe486ab1d9e162769c3ec07e0f8594c42260 Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Fri, 1 Sep 2023 13:18:16 +0300 Subject: [PATCH 06/16] Add swift swaps requests visualisation to schedule (#2881) # What this PR does Add swift swaps requests visualisation to schedule ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/2844 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 + grafana-plugin/.eslintrc.js | 1 + .../src/components/Avatar/Avatar.module.css | 7 +- .../src/components/Avatar/Avatar.tsx | 2 +- .../src/components/Text/Text.module.scss | 4 + grafana-plugin/src/components/Text/Text.tsx | 2 +- .../EscalationVariants/EscalationVariants.tsx | 2 +- .../containers/Rotation/Rotation.module.css | 8 - .../containers/RotationForm/ShiftSwapForm.tsx | 64 ++- .../containers/Rotations/Rotations.module.css | 12 +- .../src/containers/Rotations/Rotations.tsx | 26 +- .../containers/Rotations/ScheduleFinal.tsx | 4 +- .../Rotations/ScheduleOverrides.tsx | 103 +++-- .../ScheduleSlot/ScheduleSlot.module.css | 15 + .../containers/ScheduleSlot/ScheduleSlot.tsx | 371 +++++++++++++----- .../src/models/schedule/schedule.helpers.ts | 35 +- .../src/models/schedule/schedule.ts | 97 ++++- .../src/models/schedule/schedule.types.ts | 4 +- .../src/pages/incident/parts/PagedUsers.tsx | 2 +- .../src/pages/schedule/Schedule.tsx | 23 +- .../src/pages/schedules/Schedules.tsx | 2 +- 21 files changed, 573 insertions(+), 215 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b929ab74..6f9d1f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Switch engine to alpine base image ([2872](https://github.com/grafana/oncall/pull/2872)) +### Added + +- Visualization of shift swap requests in Overrides and swaps section ([#2844](https://github.com/grafana/oncall/issues/2844)) + ### Fixed - Address bug when a Shift Swap Request is accepted either via the web or mobile UI, and the Slack message is not diff --git a/grafana-plugin/.eslintrc.js b/grafana-plugin/.eslintrc.js index 99a8a3ce..46f9f165 100644 --- a/grafana-plugin/.eslintrc.js +++ b/grafana-plugin/.eslintrc.js @@ -55,6 +55,7 @@ module.exports = { * https://github.com/jsx-eslint/eslint-plugin-react/issues/3325 */ 'react/prop-types': 'off', + 'react/no-unused-prop-types': 'off', 'react/jsx-key': 'warn', 'react/jsx-no-target-blank': 'warn', 'react/no-unescaped-entities': 'off', diff --git a/grafana-plugin/src/components/Avatar/Avatar.module.css b/grafana-plugin/src/components/Avatar/Avatar.module.css index be93bd70..767da1ba 100644 --- a/grafana-plugin/src/components/Avatar/Avatar.module.css +++ b/grafana-plugin/src/components/Avatar/Avatar.module.css @@ -4,12 +4,17 @@ overflow: hidden; } +.avatarSize-xs { + width: 12px; + height: 12px; +} + .avatarSize-small { width: 16px; height: 16px; } -.avatarSize-big { +.avatarSize-medium { width: 24px; height: 24px; } diff --git a/grafana-plugin/src/components/Avatar/Avatar.tsx b/grafana-plugin/src/components/Avatar/Avatar.tsx index 85c7a3c6..63dd7f44 100644 --- a/grafana-plugin/src/components/Avatar/Avatar.tsx +++ b/grafana-plugin/src/components/Avatar/Avatar.tsx @@ -6,7 +6,7 @@ import styles from './Avatar.module.css'; interface AvatarProps { src: string; - size: string; + size: 'xs' | 'small' | 'medium' | 'large'; className?: string; } diff --git a/grafana-plugin/src/components/Text/Text.module.scss b/grafana-plugin/src/components/Text/Text.module.scss index ffbac9bf..7f6a0318 100644 --- a/grafana-plugin/src/components/Text/Text.module.scss +++ b/grafana-plugin/src/components/Text/Text.module.scss @@ -39,6 +39,10 @@ text-decoration: underline; } + &--xs { + font-size: 8px; + } + &--small { font-size: 12px; } diff --git a/grafana-plugin/src/components/Text/Text.tsx b/grafana-plugin/src/components/Text/Text.tsx index aa939a96..49a052d0 100644 --- a/grafana-plugin/src/components/Text/Text.tsx +++ b/grafana-plugin/src/components/Text/Text.tsx @@ -14,7 +14,7 @@ interface TextProps extends HTMLAttributes { type?: TextType; strong?: boolean; underline?: boolean; - size?: 'small' | 'medium' | 'large'; + size?: 'xs' | 'small' | 'medium' | 'large'; keyboard?: boolean; className?: string; wrap?: boolean; diff --git a/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx b/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx index 6aa0f9a1..08f05cb7 100644 --- a/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx +++ b/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx @@ -189,7 +189,7 @@ const UserResponder = ({ important, data, onImportantChange, handleDelete }) =>
- +
{data?.username} {data.notification_chain_verbal.default || data.notification_chain_verbal.important ? ( diff --git a/grafana-plugin/src/containers/Rotation/Rotation.module.css b/grafana-plugin/src/containers/Rotation/Rotation.module.css index 5d462ddf..805ef3c8 100644 --- a/grafana-plugin/src/containers/Rotation/Rotation.module.css +++ b/grafana-plugin/src/containers/Rotation/Rotation.module.css @@ -12,14 +12,6 @@ width: 100%; } -.root:first-child { - padding-top: 26px; -} - -.root:last-child { - padding-bottom: 26px; -} - .root:hover { background: var(--secondary-background); } diff --git a/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx b/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx index b410a994..f750ccae 100644 --- a/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx @@ -28,6 +28,7 @@ const cx = cn.bind(styles); interface ShiftSwapFormProps { id: ShiftSwap['id'] | 'new'; scheduleId: Schedule['id']; + startMoment: dayjs.Dayjs; params: Partial; currentTimezone: Timezone; @@ -37,12 +38,15 @@ interface ShiftSwapFormProps { } const ShiftSwapForm = (props: ShiftSwapFormProps) => { - const { onUpdate, onHide, id, scheduleId, params: defaultParams, currentTimezone } = props; + const { onUpdate, onHide, id, scheduleId, startMoment, params: defaultParams, currentTimezone } = props; const [shiftSwap, setShiftSwap] = useState({ ...defaultParams }); const store = useStore(); - const { scheduleStore } = store; + const { + scheduleStore, + userStore: { currentUserPk }, + } = store; useEffect(() => { if (id !== 'new') { @@ -50,6 +54,12 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { } }, [id]); + const handleHide = useCallback(() => { + scheduleStore.clearPreview(); + + onHide(); + }, []); + useEffect(() => { if (defaultParams) { setShiftSwap({ ...shiftSwap, swap_start: defaultParams.swap_start, swap_end: defaultParams.swap_end }); @@ -58,7 +68,9 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { const handleShiftSwapStartChange = useCallback( (value) => { - setShiftSwap({ ...shiftSwap, swap_start: getUTCString(value) }); + const diff = dayjs(shiftSwap.swap_end).diff(dayjs(shiftSwap.swap_start)); + + setShiftSwap({ ...shiftSwap, swap_start: getUTCString(value), swap_end: getUTCString(value.add(diff)) }); }, [shiftSwap] ); @@ -70,6 +82,16 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { [shiftSwap] ); + useEffect(() => { + if (id === 'new') { + store.scheduleStore.updateShiftsSwapPreview(scheduleId, startMoment, { + id: 'new', + beneficiary: currentUserPk, + ...shiftSwap, + }); + } + }, [shiftSwap, startMoment]); + const handleDescriptionChange = useCallback( (event) => { setShiftSwap({ ...shiftSwap, description: event.target.value }); @@ -98,6 +120,12 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { onUpdate(); }, [id]); + useEffect(() => { + if (shiftSwap?.beneficiary && !store.userStore.items[shiftSwap.beneficiary]) { + store.userStore.updateItem(shiftSwap.beneficiary); + } + }, [shiftSwap?.beneficiary]); + const beneficiaryName = shiftSwap?.beneficiary && store.userStore.items[shiftSwap.beneficiary]?.name; const isNew = id === 'new'; @@ -108,7 +136,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { top="0" isOpen width="430px" - onDismiss={onHide} + onDismiss={handleHide} contentElement={(props, children) => (
{children}
@@ -120,26 +148,30 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { {isNew && New} - - Shift swap - + {isNew ? 'Shift swap request' : 'Shift swap'} {!isNew && ( - + )} - +
{!isNew && ( - + )} @@ -169,7 +201,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { {!isNew && ( - + {shiftSwap?.benefactor ? ( { shiftEnd={shiftSwap.swap_end} /> ) : ( - Not taken yet + Not accepted yet )} )} @@ -193,8 +225,12 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { Create ) : ( - )} diff --git a/grafana-plugin/src/containers/Rotations/Rotations.module.css b/grafana-plugin/src/containers/Rotations/Rotations.module.css index 7cb2f5d5..b764648c 100644 --- a/grafana-plugin/src/containers/Rotations/Rotations.module.css +++ b/grafana-plugin/src/containers/Rotations/Rotations.module.css @@ -24,11 +24,6 @@ margin: 16px 0; } -.rotations-plus-title { - display: flex; - flex-direction: column; -} - .layer { display: block; } @@ -49,8 +44,15 @@ background: rgba(204, 204, 220, 0.12); } +.rotations-plus-title { + display: flex; + flex-direction: column; +} + .header-plus-content { position: relative; + padding-top: 26px; + padding-bottom: 26px; } .layer-header { diff --git a/grafana-plugin/src/containers/Rotations/Rotations.tsx b/grafana-plugin/src/containers/Rotations/Rotations.tsx index c1a3725f..85063bed 100644 --- a/grafana-plugin/src/containers/Rotations/Rotations.tsx +++ b/grafana-plugin/src/containers/Rotations/Rotations.tsx @@ -17,13 +17,12 @@ import { getColor, getLayersFromStore } from 'models/schedule/schedule.helpers'; import { Layer, Schedule, ScheduleType, Shift, ShiftSwap, Event } from 'models/schedule/schedule.types'; import { Timezone } from 'models/timezone/timezone.types'; import { User } from 'models/user/user.types'; -import { getUTCString } from 'pages/schedule/Schedule.helpers'; import { WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; import { UserActions } from 'utils/authorization'; import { DEFAULT_TRANSITION_TIMEOUT } from './Rotations.config'; -import { findClosestUserEvent, findColor } from './Rotations.helpers'; +import { findColor } from './Rotations.helpers'; import styles from './Rotations.module.css'; @@ -75,9 +74,6 @@ class Rotations extends Component { filters, onShowShiftSwapForm, onSlotClick, - store: { - userStore: { currentUserPk }, - }, } = this.props; const { layerPriority, shiftStartToShowRotationForm, shiftEndToShowRotationForm } = this.state; @@ -115,24 +111,6 @@ class Rotations extends Component {
- {disabled ? ( isTypeReadOnly ? ( @@ -176,7 +154,7 @@ class Rotations extends Component { Layer {layer.priority} -
+
{!currentTimeHidden && (
diff --git a/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx b/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx index 59685916..9ad69290 100644 --- a/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx +++ b/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx @@ -11,7 +11,7 @@ import Text from 'components/Text/Text'; import TimelineMarks from 'components/TimelineMarks/TimelineMarks'; import Rotation from 'containers/Rotation/Rotation'; import { - flattenFinalShifs, + flattenShiftEvents, getLayersFromStore, getOverridesFromStore, getShiftsFromStore, @@ -60,7 +60,7 @@ class ScheduleFinal extends Component void; + onShowShiftSwapForm: (id: ShiftSwap['id'] | 'new', params?: Partial) => void; onCreate: () => void; onUpdate: () => void; onDelete: () => void; @@ -67,12 +74,20 @@ class ScheduleOverrides extends Component
- Overrides + Overrides and swaps
- {isTypeReadOnly ? ( - -
- + {isTypeReadOnly ? ( + +
+ +
+
+ ) : ( + + -
-
- ) : ( - - - - )} + + )} +
{!currentTimeHidden &&
} + + {shiftSwaps && shiftSwaps.length + ? shiftSwaps.map(({ isPreview, events }, index) => ( + + { + if (event.is_gap) { + return; + } + onShowShiftSwapForm(event.shiftSwapId); + }} + transparent={isPreview} + filters={filters} + /> + + )) + : null} + {shifts && shifts.length ? ( - shifts.map(({ shiftId, isPreview, events }, rotationIndex) => ( - + shifts.map(({ shiftId, isPreview, events }, index) => ( + { @@ -136,6 +194,7 @@ class ScheduleOverrides extends Component = observer((props) => { filters, onClick, } = props; - const { users } = event; - - const getShiftSwapClickHandler = useCallback((swapId: ShiftSwap['id']) => { - return (event: React.MouseEvent) => { - event.stopPropagation(); - - onShiftSwapClick(swapId); - }; - }, []); const start = dayjs(event.start); const end = dayjs(event.end); const duration = end.diff(start, 'seconds'); - const store = useStore(); - const base = 60 * 60 * 24 * 7; const width = duration / base; - const onCallNow = store.scheduleStore.items[scheduleId]?.on_call_now; + const currentMoment = useMemo(() => dayjs(), []); - const enableWebOverrides = store.scheduleStore.items[scheduleId]?.enable_web_overrides; + const renderEvent = (event): React.ReactElement | React.ReactElement[] => { + if (event.shiftSwapId) { + return ( + + ); + } - return ( -
- {event.is_gap ? ( + if (event.is_gap) { + return ( }>
- ) : event.is_empty ? ( + ); + } + + if (event.is_empty) { + return (
- ) : ( - users.map(({ display_name, pk: userPk, swap_request }) => { - const storeUser = store.userStore.items[userPk]; + ); + } - const isCurrentUserSlot = userPk === store.userStore.currentUserPk; - const inactive = filters && filters.users.length && !filters.users.includes(userPk); + return ( + + ); + }; - const title = storeUser ? getTitle(storeUser) : display_name; - - const isOncall = Boolean( - storeUser && onCallNow && onCallNow.some((onCallUser) => storeUser.pk === onCallUser.pk) - ); - - const isShiftSwap = Boolean(swap_request); - - let backgroundColor = color; - if (isShiftSwap) { - backgroundColor = SHIFT_SWAP_COLOR; - } - - const scheduleSlotContent = ( -
- {storeUser && (!swap_request || swap_request.user) && ( - - )} -
- {swap_request && !swap_request.user ? : title} -
-
- ); - - if (!storeUser) { - return scheduleSlotContent; - } // show without a tooltip as we're lacking user info - - return ( - - } - > - {scheduleSlotContent} - - ); - }) - )} + return ( +
+ {renderEvent(event)}
); }); export default ScheduleSlot; +interface ShiftSwapEventProps { + event: Event; + currentTimezone: Timezone; + simplified: boolean; + currentMoment: dayjs.Dayjs; +} + +const ShiftSwapEvent = (props: ShiftSwapEventProps) => { + const { event, currentTimezone, simplified, currentMoment } = props; + + const store = useStore(); + + const shiftSwap = store.scheduleStore.shiftSwaps[event.shiftSwapId]; + + useEffect(() => { + if (shiftSwap?.beneficiary && !store.userStore.items[shiftSwap.beneficiary]) { + store.userStore.updateItem(shiftSwap.beneficiary); + } + }, [shiftSwap?.beneficiary]); + + useEffect(() => { + if (shiftSwap?.benefactor && !store.userStore.items[shiftSwap.benefactor]) { + store.userStore.updateItem(shiftSwap.benefactor); + } + }, [shiftSwap?.benefactor]); + + const beneficiary = store.userStore.items[shiftSwap?.beneficiary]; + const benefactor = store.userStore.items[shiftSwap?.benefactor]; + + const scheduleSlotContent = ( +
+ {shiftSwap && ( + + {beneficiary && } + {benefactor ? ( + + ) : ( +
+ + ? + +
+ )} +
+ )} +
+ ); + + if (!shiftSwap) { + return scheduleSlotContent; + } + + return ( + + } + > + {scheduleSlotContent} + + ); +}; + +interface RegularEventProps { + event: Event; + scheduleId: Schedule['id']; + currentTimezone: Timezone; + handleAddOverride: (event: React.MouseEvent) => void; + handleAddShiftSwap: (event: React.MouseEvent) => void; + onShiftSwapClick: (id: ShiftSwap['id']) => void; + simplified: boolean; + color?: string; + filters?: ScheduleFiltersType; + start: dayjs.Dayjs; + duration: number; + currentMoment: dayjs.Dayjs; +} + +const RegularEvent = (props: RegularEventProps) => { + const { + event, + scheduleId, + onShiftSwapClick, + filters, + color, + currentTimezone, + simplified, + start, + duration, + handleAddOverride, + handleAddShiftSwap, + currentMoment, + } = props; + const store = useStore(); + + const { users } = event; + + const getShiftSwapClickHandler = useCallback( + (swapId: ShiftSwap['id']) => { + return (event: React.MouseEvent) => { + event.stopPropagation(); + + onShiftSwapClick(swapId); + }; + }, + [onShiftSwapClick] + ); + + const onCallNow = store.scheduleStore.items[scheduleId]?.on_call_now; + + const enableWebOverrides = store.scheduleStore.items[scheduleId]?.enable_web_overrides; + + return ( + <> + {users.map(({ display_name, pk: userPk, swap_request }) => { + const storeUser = store.userStore.items[userPk]; + + const isCurrentUserSlot = userPk === store.userStore.currentUserPk; + const inactive = filters && filters.users.length && !filters.users.includes(userPk); + + const title = storeUser ? getTitle(storeUser) : display_name; + + const isOncall = Boolean( + storeUser && onCallNow && onCallNow.some((onCallUser) => storeUser.pk === onCallUser.pk) + ); + + const isShiftSwap = Boolean(swap_request); + + let backgroundColor = color; + if (isShiftSwap) { + backgroundColor = SHIFT_SWAP_COLOR; + } + + const scheduleSlotContent = ( +
+ {storeUser && (!swap_request || swap_request.user) && ( + + )} +
+ {swap_request && !swap_request.user ? : title} +
+
+ ); + + if (!storeUser) { + return scheduleSlotContent; + } // show without a tooltip as we're lacking user info + + return ( + + } + > + {scheduleSlotContent} + + ); + })} + + ); +}; + interface ScheduleSlotDetailsProps { user: User; - isOncall: boolean; + isOncall?: boolean; currentTimezone: Timezone; event: Event; - handleAddOverride: (event: React.SyntheticEvent) => void; - handleAddShiftSwap: (event: React.SyntheticEvent) => void; + handleAddOverride?: (event: React.SyntheticEvent) => void; + handleAddShiftSwap?: (event: React.SyntheticEvent) => void; simplified?: boolean; color: string; isShiftSwap?: boolean; beneficiaryName?: string; benefactorName?: string; + currentMoment: dayjs.Dayjs; } const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { @@ -195,13 +359,12 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { isShiftSwap, beneficiaryName, benefactorName, + currentMoment, } = props; const store = useStore(); const { scheduleStore } = store; - const currentMoment = useMemo(() => dayjs(), []); - const shift = scheduleStore.shifts[event.shift?.pk]; return ( @@ -223,15 +386,15 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { Swap pair - {beneficiaryName} (creator) + {beneficiaryName} (requested by) {benefactorName ? ( - {benefactorName} (taken by) + {benefactorName} (accepted by) ) : ( - Not taken yet + Not accepted yet )} @@ -248,8 +411,8 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { User local time
- {currentMoment.tz(user.timezone).format('DD MMM, HH:mm')} -
({getTzOffsetString(currentMoment.tz(user.timezone))}) + {currentMoment.tz(user?.timezone).format('DD MMM, HH:mm')} +
({getTzOffsetString(currentMoment.tz(user?.timezone))})
Current timezone diff --git a/grafana-plugin/src/models/schedule/schedule.helpers.ts b/grafana-plugin/src/models/schedule/schedule.helpers.ts index 41125388..0b92b8ef 100644 --- a/grafana-plugin/src/models/schedule/schedule.helpers.ts +++ b/grafana-plugin/src/models/schedule/schedule.helpers.ts @@ -2,7 +2,7 @@ import dayjs from 'dayjs'; import { RootStore } from 'state'; -import { Event, Layer, Schedule, ScheduleType, Shift, ShiftEvents } from './schedule.types'; +import { Event, Layer, Schedule, ScheduleType, Shift, ShiftEvents, ShiftSwap } from './schedule.types'; export const getFromString = (moment: dayjs.Dayjs) => { return moment.format('YYYY-MM-DD'); @@ -25,6 +25,25 @@ const createGap = (start, end) => { }; }; +export const createShiftSwapEventFromShiftSwap = (shiftSwap: Partial) => { + return { + shiftSwapId: shiftSwap.id, + start: shiftSwap.swap_start, + end: shiftSwap.swap_end, + is_gap: false, + users: [], + all_day: false, + shift: null, + missing_users: [], + is_empty: true, + is_shift_swap: true, + calendar_type: ScheduleType.API, + priority_level: null, + source: 'web', + is_override: false, + }; +}; + export const fillGaps = (events: Event[]) => { const newEvents = []; @@ -74,7 +93,7 @@ export const getShiftsFromStore = ( : (store.scheduleStore.events[scheduleId]?.['final']?.[getFromString(startMoment)] as any); }; -export const flattenFinalShifs = (shifts: ShiftEvents[]) => { +export const flattenShiftEvents = (shifts: ShiftEvents[]) => { if (!shifts) { return undefined; } @@ -193,6 +212,16 @@ export const getLayersFromStore = (store: RootStore, scheduleId: Schedule['id'], : (store.scheduleStore.events[scheduleId]?.['rotation']?.[getFromString(startMoment)] as Layer[]); }; +export const getShiftSwapsFromStore = ( + store: RootStore, + scheduleId: Schedule['id'], + startMoment: dayjs.Dayjs +): ShiftEvents[] => { + return store.scheduleStore.shiftSwapsPreview + ? store.scheduleStore.shiftSwapsPreview[getFromString(startMoment)] + : store.scheduleStore.scheduleAndDateToShiftSwaps[scheduleId]?.[getFromString(startMoment)]; +}; + export const getOverridesFromStore = ( store: RootStore, scheduleId: Schedule['id'], @@ -200,7 +229,7 @@ export const getOverridesFromStore = ( ): ShiftEvents[] => { return store.scheduleStore.overridePreview ? store.scheduleStore.overridePreview[getFromString(startMoment)] - : (store.scheduleStore.events[scheduleId]?.['override']?.[getFromString(startMoment)] as Layer[]); + : (store.scheduleStore.events[scheduleId]?.['override']?.[getFromString(startMoment)] as ShiftEvents[]); }; export const splitToLayers = ( diff --git a/grafana-plugin/src/models/schedule/schedule.ts b/grafana-plugin/src/models/schedule/schedule.ts index 7fb49671..8219d57e 100644 --- a/grafana-plugin/src/models/schedule/schedule.ts +++ b/grafana-plugin/src/models/schedule/schedule.ts @@ -9,8 +9,10 @@ import { RootStore } from 'state'; import { SelectOption } from 'state/types'; import { + createShiftSwapEventFromShiftSwap, enrichLayers, enrichOverrides, + flattenShiftEvents, getFromString, splitToLayers, splitToShiftsAndFillGaps, @@ -48,6 +50,9 @@ export class ScheduleStore extends BaseStore { @observable.shallow shiftSwaps: { [id: string]: ShiftSwap } = {}; + @observable.shallow + scheduleAndDateToShiftSwaps: { [scheduleId: string]: { [date: string]: ShiftEvents[] } } = {}; + @observable.shallow rotations: { [id: string]: { @@ -65,13 +70,18 @@ export class ScheduleStore extends BaseStore { } = {}; @observable - finalPreview?: Array<{ shiftId: Shift['id']; events: Event[] }>; + finalPreview?: { [fromString: string]: Array<{ shiftId: Shift['id']; events: Event[] }> }; @observable - rotationPreview?: Layer[]; + rotationPreview?: { [fromString: string]: Layer[] }; @observable - overridePreview?: Array<{ shiftId: Shift['id']; isPreview?: boolean; events: Event[] }>; + shiftSwapsPreview?: { + [fromString: string]: ShiftEvents[]; + }; + + @observable + overridePreview?: { [fromString: string]: ShiftEvents[] }; @observable rotationFormLiveParams: RotationFormLiveParams = undefined; @@ -105,24 +115,6 @@ export class ScheduleStore extends BaseStore { return schedule; } - @action - async updateScheduleEvents( - scheduleId: Schedule['id'], - withEmpty: boolean, - with_gap: boolean, - date: string, - user_tz: string - ) { - const { events } = await makeRequest(`/schedules/${scheduleId}/events/`, { - params: { date, user_tz, with_empty: withEmpty, with_gap: with_gap }, - }); - - this.scheduleToScheduleEvents = { - ...this.scheduleToScheduleEvents, - [scheduleId]: events, - }; - } - @action async updateItems( f: RemoteFiltersType | string = { searchTerm: '', type: undefined, used: undefined }, @@ -283,11 +275,36 @@ export class ScheduleStore extends BaseStore { this.finalPreview = { ...this.finalPreview, [fromString]: splitToShiftsAndFillGaps(response.final) }; } + @action + async updateShiftsSwapPreview(scheduleId: Schedule['id'], startMoment: dayjs.Dayjs, params: Partial) { + const fromString = getFromString(startMoment); + + const newShiftEvents: ShiftEvents = { + shiftId: 'new', + events: [createShiftSwapEventFromShiftSwap(params)], + isPreview: true, + }; + + if (!this.scheduleAndDateToShiftSwaps[scheduleId][fromString]) { + await this.updateShiftSwaps(scheduleId, startMoment); + } + + const existingShiftEventsList: ShiftEvents[] = this.scheduleAndDateToShiftSwaps[scheduleId][fromString]; + + const shiftEventsListFlattened = flattenShiftEvents([...existingShiftEventsList, newShiftEvents]); + + this.shiftSwapsPreview = { + ...this.shiftSwapsPreview, + [fromString]: shiftEventsListFlattened, + }; + } + @action clearPreview() { this.finalPreview = undefined; this.rotationPreview = undefined; this.overridePreview = undefined; + this.shiftSwapsPreview = undefined; this.rotationFormLiveParams = undefined; } @@ -456,4 +473,42 @@ export class ScheduleStore extends BaseStore { return result; } + + async updateShiftSwaps(scheduleId: Schedule['id'], startMoment: dayjs.Dayjs, days = 9) { + const fromString = getFromString(startMoment); + + const dayBefore = startMoment.subtract(1, 'day'); + + const result = await makeRequest(`/schedules/${scheduleId}/filter_shift_swaps/`, { + method: 'GET', + params: { + date: getFromString(dayBefore), + days, + }, + }); + + const shiftEventsList: ShiftEvents[] = result.shift_swaps.map((shiftSwap) => ({ + shiftId: shiftSwap.id, + events: [createShiftSwapEventFromShiftSwap(shiftSwap)], + isPreview: false, + })); + + const shiftEventsListFlattened = flattenShiftEvents(shiftEventsList); + + this.shiftSwaps = result.shift_swaps.reduce( + (memo, shiftSwap) => ({ + ...memo, + [shiftSwap.id]: shiftSwap, + }), + this.shiftSwaps + ); + + this.scheduleAndDateToShiftSwaps = { + ...this.scheduleAndDateToShiftSwaps, + [scheduleId]: { + ...this.scheduleAndDateToShiftSwaps[scheduleId], + [fromString]: shiftEventsListFlattened, + }, + }; + } } diff --git a/grafana-plugin/src/models/schedule/schedule.types.ts b/grafana-plugin/src/models/schedule/schedule.types.ts index 0e57de64..670109cc 100644 --- a/grafana-plugin/src/models/schedule/schedule.types.ts +++ b/grafana-plugin/src/models/schedule/schedule.types.ts @@ -103,6 +103,8 @@ export interface Event { swap_request?: SwapRequest; }>; is_override: boolean; + + shiftSwapId?: ShiftSwap['id']; // if event is acually shift swap request (filled out by frontend) } export interface Events { @@ -120,7 +122,7 @@ export interface Layer { export interface ShiftEvents { shiftId: string; events: Event[]; - priority: number; + priority?: number; isPreview?: boolean; } diff --git a/grafana-plugin/src/pages/incident/parts/PagedUsers.tsx b/grafana-plugin/src/pages/incident/parts/PagedUsers.tsx index 12a0c18b..a0640406 100644 --- a/grafana-plugin/src/pages/incident/parts/PagedUsers.tsx +++ b/grafana-plugin/src/pages/incident/parts/PagedUsers.tsx @@ -62,7 +62,7 @@ const PagedUsers = observer((props: PagedUsersProps) => {
  • - + {pagedUser.username} {Boolean( storeUser && diff --git a/grafana-plugin/src/pages/schedule/Schedule.tsx b/grafana-plugin/src/pages/schedule/Schedule.tsx index 5271e984..9bbb3993 100644 --- a/grafana-plugin/src/pages/schedule/Schedule.tsx +++ b/grafana-plugin/src/pages/schedule/Schedule.tsx @@ -140,13 +140,15 @@ class SchedulePage extends React.Component !isUserActionAllowed(UserActions.SchedulesWrite) || schedule?.type !== ScheduleType.API || !!shiftIdToShowRotationForm || - shiftIdToShowOverridesForm; + shiftIdToShowOverridesForm || + shiftSwapIdToShowForm; const disabledOverrideForm = !isUserActionAllowed(UserActions.SchedulesWrite) || !schedule?.enable_web_overrides || !!shiftIdToShowOverridesForm || - shiftIdToShowRotationForm; + shiftIdToShowRotationForm || + shiftSwapIdToShowForm; return ( @@ -272,7 +274,7 @@ class SchedulePage extends React.Component disabled={disabledRotationForm} onShowOverrideForm={this.handleShowOverridesForm} filters={filters} - onShowShiftSwapForm={this.handleShowShiftSwapForm} + onShowShiftSwapForm={!shiftSwapIdToShowForm ? this.handleShowShiftSwapForm : undefined} onSlotClick={ shiftSwapIdToShowForm ? this.adjustShiftSwapForm @@ -293,7 +295,7 @@ class SchedulePage extends React.Component onShowOverrideForm={this.handleShowOverridesForm} disabled={disabledRotationForm} filters={filters} - onShowShiftSwapForm={this.handleShowShiftSwapForm} + onShowShiftSwapForm={!shiftSwapIdToShowForm ? this.handleShowShiftSwapForm : undefined} onSlotClick={shiftSwapIdToShowForm ? this.adjustShiftSwapForm : undefined} /> disabled={disabledOverrideForm} shiftStartToShowOverrideForm={shiftStartToShowOverrideForm} shiftEndToShowOverrideForm={shiftEndToShowOverrideForm} + onShowShiftSwapForm={!shiftSwapIdToShowForm ? this.handleShowShiftSwapForm : undefined} filters={filters} />
  • @@ -337,10 +340,11 @@ class SchedulePage extends React.Component )} @@ -426,6 +430,7 @@ class SchedulePage extends React.Component store.scheduleStore.updateEvents(scheduleId, startMoment, 'rotation'), store.scheduleStore.updateEvents(scheduleId, startMoment, 'override'), store.scheduleStore.updateEvents(scheduleId, startMoment, 'final'), + store.scheduleStore.updateShiftSwaps(scheduleId, startMoment), ]); }; @@ -453,6 +458,14 @@ class SchedulePage extends React.Component }); }; + handleUpdateShiftSwaps = () => { + const { store } = this.props; + + this.updateEvents().then(() => { + store.scheduleStore.clearPreview(); + }); + }; + handleDeleteRotation = () => { const { store } = this.props; diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index 99ae23ac..7fc5950f 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -369,7 +369,7 @@ class SchedulesPage extends React.Component
    - +
    {user.username} From 361d45dd02f649ba4b18d53b52510ecf91696467 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Fri, 1 Sep 2023 12:48:47 +0200 Subject: [PATCH 07/16] Clean up check escalation finished task (#2943) # What this PR does Clean up check escalation finished task, update description ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../alerts/tasks/check_escalation_finished.py | 31 +++++-------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/engine/apps/alerts/tasks/check_escalation_finished.py b/engine/apps/alerts/tasks/check_escalation_finished.py index 80c8d1bf..f538b3e5 100644 --- a/engine/apps/alerts/tasks/check_escalation_finished.py +++ b/engine/apps/alerts/tasks/check_escalation_finished.py @@ -81,36 +81,19 @@ def audit_alert_group_escalation(alert_group: "AlertGroup") -> None: f"{base_msg}'s escalation snapshot has {num_of_executed_escalation_policy_snapshots} executed escalation policies" ) - # TODO: consider adding the below checks later on. This is it a bit trickier to properly audit as the - # number of log records can vary if there are any STEP_NOTIFY_IF_NUM_ALERTS_IN_TIME_WINDOW or - # STEP_REPEAT_ESCALATION_N_TIMES escalation policy steps in the escalation chain - # see conversations in the original PR (https://github.com/grafana/oncall/pull/1266) for more context on this - # - # compare number of triggered/failed alert group log records to the number of executed - # escalation policy snapshot steps - # num_of_relevant_log_records = AlertGroupLogRecord.objects.filter( - # alert_group_id=alert_group_id, - # type__in=[AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED, AlertGroupLogRecord.TYPE_ESCALATION_FAILED], - # ).count() - - # if num_of_relevant_log_records < num_of_executed_escalation_policy_snapshots: - # raise AlertGroupEscalationPolicyExecutionAuditException( - # f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is less " - # f"than the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})" - # ) - - # task_logger.info( - # f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is greater " - # f"than or equal to the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})" - # ) - task_logger.info(f"{base_msg} passed the audit checks") @shared_task def check_escalation_finished_task() -> None: """ - don't retry this task, the idea is to be alerted of failures + This task takes alert groups with active escalation, checks if escalation snapshot with escalation policies + was created and next escalation step eta is higher than now minus 5 min for every active alert group, + what means that escalations are going as expected. + If there are alert groups that failed the check, it raises exception. Otherwise - send heartbeat. Missing heartbeat + raises alert. + + Attention: don't retry this task, the idea is to be alerted of failures """ from apps.alerts.models import AlertGroup From 619cb42a3a0707d1f7b7692724c4d3afab808586 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Sep 2023 13:54:37 +0200 Subject: [PATCH 08/16] Remove apps.mobile_app.tasks.notify_user_async celery task (#2942) # What this PR does This task was renamed in a v1.3.30. It is no longer referenced and there are no pending tasks in production queues: ![Screenshot 2023-09-01 at 09 35 45](https://github.com/grafana/oncall/assets/9406895/0f7149bd-efe3-401e-ac94-a664af67472a) ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/mobile_app/tasks/__init__.py | 2 +- engine/apps/mobile_app/tasks/new_alert_group.py | 6 ------ engine/common/tests/test_task_queue_assignment.py | 1 - engine/settings/celery_task_routes.py | 2 -- 4 files changed, 1 insertion(+), 10 deletions(-) diff --git a/engine/apps/mobile_app/tasks/__init__.py b/engine/apps/mobile_app/tasks/__init__.py index 42e04767..608a639e 100644 --- a/engine/apps/mobile_app/tasks/__init__.py +++ b/engine/apps/mobile_app/tasks/__init__.py @@ -2,7 +2,7 @@ from .going_oncall_notification import ( # noqa:F401 conditionally_send_going_oncall_push_notifications_for_all_schedules, conditionally_send_going_oncall_push_notifications_for_schedule, ) -from .new_alert_group import notify_user_about_new_alert_group, notify_user_async # noqa:F401 +from .new_alert_group import notify_user_about_new_alert_group # noqa:F401 from .new_shift_swap_request import ( # noqa:F401 notify_shift_swap_request, notify_shift_swap_requests, diff --git a/engine/apps/mobile_app/tasks/new_alert_group.py b/engine/apps/mobile_app/tasks/new_alert_group.py index eb211127..715d6300 100644 --- a/engine/apps/mobile_app/tasks/new_alert_group.py +++ b/engine/apps/mobile_app/tasks/new_alert_group.py @@ -137,9 +137,3 @@ def notify_user_about_new_alert_group(user_pk, alert_group_pk, notification_poli message = _get_fcm_message(alert_group, user, device_to_notify, critical) send_push_notification(device_to_notify, message, _create_error_log_record) - - -# TODO: remove this in a future release -@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=MAX_RETRIES) -def notify_user_async(user_pk, alert_group_pk, notification_policy_pk, critical): - notify_user_about_new_alert_group(user_pk, alert_group_pk, notification_policy_pk, critical) diff --git a/engine/common/tests/test_task_queue_assignment.py b/engine/common/tests/test_task_queue_assignment.py index 1acb5a0a..a5fb202d 100644 --- a/engine/common/tests/test_task_queue_assignment.py +++ b/engine/common/tests/test_task_queue_assignment.py @@ -9,7 +9,6 @@ we should avoid @shared_dedicated_queue_retry_task or @shared_task and remove entirely if it is not needed. """ COMMON_IGNORED_TASKS = { - "apps.mobile_app.tasks.new_alert_group.notify_user_async", "apps.alerts.tasks.create_contact_points_for_datasource.schedule_create_contact_points_for_datasource", "common.oncall_gateway.tasks.create_slack_connector_async_v2", } diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index 2e4f6e4d..d7075adf 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -100,8 +100,6 @@ CELERY_TASK_ROUTES = { "apps.integrations.tasks.create_alert": {"queue": "critical"}, "apps.integrations.tasks.create_alertmanager_alerts": {"queue": "critical"}, "apps.integrations.tasks.start_notify_about_integration_ratelimit": {"queue": "critical"}, - # TODO: remove apps.mobile_app.tasks.notify_user_async in a future release - "apps.mobile_app.tasks.notify_user_async": {"queue": "critical"}, "apps.mobile_app.tasks.new_alert_group.notify_user_about_new_alert_group": {"queue": "critical"}, "apps.mobile_app.tasks.going_oncall_notification.conditionally_send_going_oncall_push_notifications_for_schedule": { "queue": "critical" From e472b03e1de1334805db9d9320704b9a4f4de0f0 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Fri, 1 Sep 2023 15:11:02 +0200 Subject: [PATCH 09/16] Remove deprecated alerting integration tasks (#2944) # What this PR does These celery tasks have not been used for more than one week (since [v1.3.25](https://github.com/grafana/oncall/releases/tag/v1.3.25) which released an improvement for Grafana Alerting integration) ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/alerts/tasks/__init__.py | 7 +- .../create_contact_points_for_datasource.py | 109 ------------------ .../sync_grafana_alerting_contact_points.py | 6 - .../tests/test_task_queue_assignment.py | 1 - engine/settings/celery_task_routes.py | 2 - 5 files changed, 1 insertion(+), 124 deletions(-) delete mode 100644 engine/apps/alerts/tasks/create_contact_points_for_datasource.py diff --git a/engine/apps/alerts/tasks/__init__.py b/engine/apps/alerts/tasks/__init__.py index 0616c528..0dce9b3a 100644 --- a/engine/apps/alerts/tasks/__init__.py +++ b/engine/apps/alerts/tasks/__init__.py @@ -4,8 +4,6 @@ from .alert_group_web_title_cache import ( # noqa:F401 update_web_title_cache_for_alert_receive_channel, ) from .check_escalation_finished import check_escalation_finished_task # noqa: F401 -from .create_contact_points_for_datasource import create_contact_points_for_datasource # noqa: F401 -from .create_contact_points_for_datasource import schedule_create_contact_points_for_datasource # noqa: F401 from .custom_button_result import custom_button_result # noqa: F401 from .custom_webhook_result import custom_webhook_result # noqa: F401 from .delete_alert_group import delete_alert_group # noqa: F401 @@ -22,9 +20,6 @@ from .resolve_by_last_step import resolve_by_last_step_task # noqa: F401 from .send_alert_group_signal import send_alert_group_signal # noqa: F401 from .send_update_log_report_signal import send_update_log_report_signal # noqa: F401 from .send_update_resolution_note_signal import send_update_resolution_note_signal # noqa: F401 -from .sync_grafana_alerting_contact_points import ( # noqa: F401 - disconnect_integration_from_alerting_contact_points, - sync_grafana_alerting_contact_points, -) +from .sync_grafana_alerting_contact_points import disconnect_integration_from_alerting_contact_points # noqa: F401 from .unsilence import unsilence_task # noqa: F401 from .wipe import wipe # noqa: F401 diff --git a/engine/apps/alerts/tasks/create_contact_points_for_datasource.py b/engine/apps/alerts/tasks/create_contact_points_for_datasource.py deleted file mode 100644 index b42b851e..00000000 --- a/engine/apps/alerts/tasks/create_contact_points_for_datasource.py +++ /dev/null @@ -1,109 +0,0 @@ -import logging - -from celery.utils.log import get_task_logger -from django.core.cache import cache -from rest_framework import status - -from common.custom_celery_tasks import shared_dedicated_queue_retry_task - -logger = get_task_logger(__name__) -logger.setLevel(logging.DEBUG) - - -def get_cache_key_create_contact_points_for_datasource(alert_receive_channel_id): - CACHE_KEY_PREFIX = "create_contact_points_for_datasource" - return f"{CACHE_KEY_PREFIX}_{alert_receive_channel_id}" - - -def set_cache_key_create_contact_points_for_datasource(alert_receive_channel_id, task_id): - CACHE_LIFETIME = 600 - cache_key = get_cache_key_create_contact_points_for_datasource(alert_receive_channel_id) - cache.set(cache_key, task_id, timeout=CACHE_LIFETIME) - - -@shared_dedicated_queue_retry_task -def schedule_create_contact_points_for_datasource(alert_receive_channel_id, datasource_list): - START_TASK_DELAY = 3 - task = create_contact_points_for_datasource.apply_async( - args=[alert_receive_channel_id, datasource_list], countdown=START_TASK_DELAY - ) - set_cache_key_create_contact_points_for_datasource(alert_receive_channel_id, task.id) - - -@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=20) -def create_contact_points_for_datasource(alert_receive_channel_id, datasource_list): - """ - Try to create contact points for other datasource. - Restart task for datasource, for which contact point was not created. - """ - cache_key = get_cache_key_create_contact_points_for_datasource(alert_receive_channel_id) - cached_task_id = cache.get(cache_key) - current_task_id = create_contact_points_for_datasource.request.id - if cached_task_id is not None and current_task_id != cached_task_id: - return - - from apps.alerts.models import AlertReceiveChannel - - alert_receive_channel = AlertReceiveChannel.objects.filter(pk=alert_receive_channel_id).first() - if not alert_receive_channel: - logger.debug( - f"Create CP task: Cannot create contact point for integration {alert_receive_channel_id}: " - f"integration does not exist" - ) - return - - grafana_alerting_sync_manager = alert_receive_channel.grafana_alerting_sync_manager - logger.debug( - f"Create CP task: Create contact points for integration {alert_receive_channel_id}, " - f"retry counter: {create_contact_points_for_datasource.request.retries}, datasource list {len(datasource_list)}" - ) - # list of datasource for which contact point creation was failed - datasources_to_create = [] - for datasource in datasource_list: - datasource_type = datasource.get("type") - logger.debug( - f"Create CP task: Create contact point for datasource {datasource_type} " - f"for integration {alert_receive_channel_id}" - ) - contact_point, response_info = grafana_alerting_sync_manager.create_contact_point(datasource) - - if contact_point is None: - if response_info.get("status_code") == status.HTTP_400_BAD_REQUEST: - logger.warning( - f"Create CP task: Failed to create contact point for integration {alert_receive_channel_id}, " - f"datasource info: {datasource}; response: {response_info}. " - f"Got 400 Bad Request, exclude from retry list." - ) - continue - logger.warning( - f"Create CP task: Failed to create contact point for integration {alert_receive_channel_id}, " - f"datasource info: {datasource}; response: {response_info}. Retrying" - ) - # Failed to create contact point. Add datasource to list and retry to create contact point for it again - datasources_to_create.append(datasource) - - # if some contact points were not created, restart task for them - if ( - datasources_to_create - and create_contact_points_for_datasource.request.retries < create_contact_points_for_datasource.max_retries - ): - logger.debug( - f"Create CP task: Retry to create contact points for integration {alert_receive_channel_id}, " - f"retry counter: {create_contact_points_for_datasource.request.retries}, " - f"datasource list {len(datasources_to_create)}" - ) - # Save task id in cache and restart the task - set_cache_key_create_contact_points_for_datasource(alert_receive_channel_id, current_task_id) - create_contact_points_for_datasource.retry(args=(alert_receive_channel_id, datasources_to_create), countdown=3) - else: - alert_receive_channel.is_finished_alerting_setup = True - alert_receive_channel.save(update_fields=["is_finished_alerting_setup"]) - logger.debug( - f"Create CP task: Alerting setup for integration {alert_receive_channel_id} is finished, " - f"retry counter: {create_contact_points_for_datasource.request.retries}, " - f"datasource list {len(datasource_list)}" - ) - logger.debug( - f"Create CP task: Finished task to create contact points for integration {alert_receive_channel_id}, " - f"datasource list {len(datasource_list)}" - ) diff --git a/engine/apps/alerts/tasks/sync_grafana_alerting_contact_points.py b/engine/apps/alerts/tasks/sync_grafana_alerting_contact_points.py index 1d13fb5f..35590b45 100644 --- a/engine/apps/alerts/tasks/sync_grafana_alerting_contact_points.py +++ b/engine/apps/alerts/tasks/sync_grafana_alerting_contact_points.py @@ -1,12 +1,6 @@ from common.custom_celery_tasks import shared_dedicated_queue_retry_task -# deprecated -@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=10) -def sync_grafana_alerting_contact_points(alert_receive_channel_id): - pass - - @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=10) def disconnect_integration_from_alerting_contact_points(alert_receive_channel_id): from apps.alerts.models import AlertReceiveChannel diff --git a/engine/common/tests/test_task_queue_assignment.py b/engine/common/tests/test_task_queue_assignment.py index a5fb202d..f81da60f 100644 --- a/engine/common/tests/test_task_queue_assignment.py +++ b/engine/common/tests/test_task_queue_assignment.py @@ -9,7 +9,6 @@ we should avoid @shared_dedicated_queue_retry_task or @shared_task and remove entirely if it is not needed. """ COMMON_IGNORED_TASKS = { - "apps.alerts.tasks.create_contact_points_for_datasource.schedule_create_contact_points_for_datasource", "common.oncall_gateway.tasks.create_slack_connector_async_v2", } diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index d7075adf..b9bb7699 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -1,10 +1,8 @@ CELERY_TASK_ROUTES = { # DEFAULT - "apps.alerts.tasks.create_contact_points_for_datasource.create_contact_points_for_datasource": {"queue": "default"}, "apps.alerts.tasks.sync_grafana_alerting_contact_points.disconnect_integration_from_alerting_contact_points": { "queue": "default" }, - "apps.alerts.tasks.sync_grafana_alerting_contact_points.sync_grafana_alerting_contact_points": {"queue": "default"}, "apps.alerts.tasks.delete_alert_group.delete_alert_group": {"queue": "default"}, "apps.alerts.tasks.invalidate_web_cache_for_alert_group.invalidate_web_cache_for_alert_group": {"queue": "default"}, "apps.alerts.tasks.send_alert_group_signal.send_alert_group_signal": {"queue": "default"}, From 5c189c8059d4460a1fe372d907fc34ed1b8f7716 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 1 Sep 2023 15:40:58 +0200 Subject: [PATCH 10/16] add create_slack_connector_async_v2 celery task to CELERY_TASK_ROUTES + remove deprecated celery tasks (#2946) # What this PR does - add `common.oncall_gateway.tasks.create_slack_connector_async_v2` celery task to `CELERY_TASK_ROUTES` - remove two deprecated tasks in `common.oncall_gateway.tasks` ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/common/oncall_gateway/tasks.py | 20 ------------------- .../tests/test_task_queue_assignment.py | 4 +--- engine/settings/celery_task_routes.py | 3 +-- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/engine/common/oncall_gateway/tasks.py b/engine/common/oncall_gateway/tasks.py index b7cfae02..d9e62897 100644 --- a/engine/common/oncall_gateway/tasks.py +++ b/engine/common/oncall_gateway/tasks.py @@ -53,26 +53,6 @@ def delete_oncall_connector_async(oncall_org_id): raise e -# deprecated -@shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), - retry_backoff=True, - max_retries=None, -) -def create_slack_connector_async(slack_id, backend): - pass - - -# deprecated -@shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), - retry_backoff=True, - max_retries=None, -) -def delete_slack_connector_async(slack_id): - pass - - @shared_dedicated_queue_retry_task( autoretry_for=(Exception,), retry_backoff=True, diff --git a/engine/common/tests/test_task_queue_assignment.py b/engine/common/tests/test_task_queue_assignment.py index f81da60f..861f6959 100644 --- a/engine/common/tests/test_task_queue_assignment.py +++ b/engine/common/tests/test_task_queue_assignment.py @@ -8,9 +8,7 @@ be added here (In development, in process of deprecation, etc.) if possible we should avoid @shared_dedicated_queue_retry_task or @shared_task and remove entirely if it is not needed. """ -COMMON_IGNORED_TASKS = { - "common.oncall_gateway.tasks.create_slack_connector_async_v2", -} +COMMON_IGNORED_TASKS = set() def check_celery_task_route_mapping(task_ids, ignored_prefixes, additional_ignored_tasks=None): diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index b9bb7699..cedbd8ec 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -9,8 +9,7 @@ CELERY_TASK_ROUTES = { "apps.alerts.tasks.wipe.wipe": {"queue": "default"}, "common.oncall_gateway.tasks.create_oncall_connector_async": {"queue": "default"}, "common.oncall_gateway.tasks.delete_oncall_connector_async": {"queue": "default"}, - "common.oncall_gateway.tasks.create_slack_connector_async": {"queue": "default"}, - "common.oncall_gateway.tasks.delete_slack_connector_async": {"queue": "default"}, + "common.oncall_gateway.tasks.create_slack_connector_async_v2": {"queue": "default"}, "common.oncall_gateway.tasks.delete_slack_connector_async_v2": {"queue": "default"}, "apps.heartbeat.tasks.integration_heartbeat_checkup": {"queue": "default"}, "apps.heartbeat.tasks.process_heartbeat_task": {"queue": "default"}, From 88c5338ea13f4c63930f70108295fc71e327a9d1 Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Fri, 1 Sep 2023 17:40:31 +0300 Subject: [PATCH 11/16] Fix cloud plugin install not refreshing (#2932) # What this PR does ## Which issue(s) this PR fixes #2874 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 7 +++++-- .../src/plugin/GrafanaPluginRootPage.tsx | 20 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f9d1f65..e9c322c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- Fix for Cloud plugin install not refreshing page after completion ([2974](https://github.com/grafana/oncall/issues/2874)) + ## v1.3.30 (2023-08-31) ### Added @@ -25,8 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix issue with helm chart when specifying `broker.type=rabbitmq` where Redis environment variables were not longer being injected by @joeyorlando ([#2927](https://github.com/grafana/oncall/pull/2927)) - Fix silence for alert groups with empty escalation chain @Ferril ([#2929](https://github.com/grafana/oncall/pull/2929)) -- Fixed NPE when migrating legacy Grafana Alerting integrations - ([#2908](https://github.com/grafana/oncall/issues/2908)) +- Fixed NPE when migrating legacy Grafana Alerting integrations ([#2908](https://github.com/grafana/oncall/issues/2908)) - Fix `IntegrityError` exceptions that occasionally would occur when trying to create `ResolutionNoteSlackMessage` objects by @joeyorlando ([#2933](https://github.com/grafana/oncall/pull/2933)) diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index 844f697e..f04b8866 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -73,7 +73,7 @@ export const Root = observer((props: AppRootProps) => { const [basicDataLoaded, setBasicDataLoaded] = useState(false); useEffect(() => { - updateBasicData(); + runQueuedUpdateData(0); }, []); const location = useLocation(); @@ -98,11 +98,6 @@ export const Root = observer((props: AppRootProps) => { }; }, []); - const updateBasicData = async () => { - await store.updateBasicData(); - setBasicDataLoaded(true); - }; - const page = getMatchedPage(location.pathname); const pagePermissionAction = pages[page]?.action; const userHasAccess = pagePermissionAction ? isUserActionAllowed(pagePermissionAction) : true; @@ -206,4 +201,17 @@ export const Root = observer((props: AppRootProps) => {
    ); + + async function runQueuedUpdateData(attemptCount: number) { + if (attemptCount === 10) { + return; + } + + try { + await store.updateBasicData(); + setBasicDataLoaded(true); + } catch { + setTimeout(() => runQueuedUpdateData(attemptCount + 1), 1000); + } + } }); From f7bdcf3d3686a67d1ddbb0e790a777a9b82ad6d6 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Fri, 1 Sep 2023 15:47:10 +0100 Subject: [PATCH 12/16] Fix `SlackMessage._alert_group` issue (#2945) # What this PR does Fixes https://github.com/grafana/oncall-private/issues/2091 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../scenarios/slack_channel_integration.py | 8 ++++- .../test_slack_channel_integration.py | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index 7a2eea9c..045ccbf7 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -1,6 +1,8 @@ import logging import typing +from django.core.exceptions import ObjectDoesNotExist + from apps.slack.scenarios import scenario_step from apps.slack.types import EventPayload, EventType, MessageEventSubtype, PayloadType, ScenarioRoute @@ -71,7 +73,11 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): except SlackMessage.DoesNotExist: return - alert_group = slack_message.get_alert_group() + try: + alert_group = slack_message.get_alert_group() + except ObjectDoesNotExist: + # SlackMessage instances without alert_group set (e.g., SSR Slack messages) + return result = self._slack_client.api_call( "chat.getPermalink", diff --git a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py index 6cc01525..05bfe06d 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py @@ -437,3 +437,36 @@ class TestSlackChannelMessageEventStep: ).count() == 0 ) + + def test_slack_message_has_no_alert_group( + self, + make_organization_and_user_with_slack_identities, + make_slack_message, + ) -> None: + """Thread messages for SlackMessage instances without alert_group set (e.g., SSR Slack messages)""" + ( + organization, + user, + slack_team_identity, + slack_user_identity, + ) = make_organization_and_user_with_slack_identities() + + channel = "potato" + ts = 88945.4849 + thread_ts = 16789.123 + + payload = { + "event": { + "channel": channel, + "ts": ts, + "thread_ts": thread_ts, + "text": "hello", + }, + } + + make_slack_message(alert_group=None, organization=organization, slack_id=thread_ts, channel_id=channel) + + step = SlackChannelMessageEventStep(slack_team_identity, organization, user) + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + assert not ResolutionNoteSlackMessage.objects.exists() From cc92c53f843c95dec86f43eae51c7a5fee8f3e10 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Mon, 4 Sep 2023 13:10:28 +0200 Subject: [PATCH 13/16] Fix build escalation snapshot (#2954) # What this PR does Fix escalation snapshot building if last notified user in escalation step "Notify users one by one (round-robin)" was deleted ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/2148 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 1 + .../serializers/escalation_policy_snapshot.py | 1 + .../alerts/tests/test_escalation_snapshot.py | 52 +++++++++++++++++++ .../apps/api/tests/test_escalation_policy.py | 3 ++ 4 files changed, 57 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9c322c8..5a648c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix for Cloud plugin install not refreshing page after completion ([2974](https://github.com/grafana/oncall/issues/2874)) +- Fix escalation snapshot building if user was deleted @Ferril ([#2954](https://github.com/grafana/oncall/pull/2954)) ## v1.3.30 (2023-08-31) diff --git a/engine/apps/alerts/escalation_snapshot/serializers/escalation_policy_snapshot.py b/engine/apps/alerts/escalation_snapshot/serializers/escalation_policy_snapshot.py index d097833e..6d648af2 100644 --- a/engine/apps/alerts/escalation_snapshot/serializers/escalation_policy_snapshot.py +++ b/engine/apps/alerts/escalation_snapshot/serializers/escalation_policy_snapshot.py @@ -64,6 +64,7 @@ class EscalationPolicySnapshotSerializer(serializers.ModelSerializer): num_alerts_in_window = serializers.IntegerField(allow_null=True, default=None) num_minutes_in_window = serializers.IntegerField(allow_null=True, default=None) pause_escalation = serializers.BooleanField(default=False) + last_notified_user = PrimaryKeyRelatedFieldWithNoneValue(allow_null=True, queryset=User.objects) class Meta: model = EscalationPolicy diff --git a/engine/apps/alerts/tests/test_escalation_snapshot.py b/engine/apps/alerts/tests/test_escalation_snapshot.py index 66003eeb..da0b6304 100644 --- a/engine/apps/alerts/tests/test_escalation_snapshot.py +++ b/engine/apps/alerts/tests/test_escalation_snapshot.py @@ -266,3 +266,55 @@ def test_escalation_snapshot_non_sequential_orders( policy_ids = [p.id for p in escalation_snapshot.executed_escalation_policy_snapshots] assert policy_ids == [step_1.id, step_2.id] + + +@pytest.mark.django_db +def test_serialize_escalation_snapshot_with_deleted_user( + make_organization_and_user, + make_user_for_organization, + make_alert_receive_channel, + make_channel_filter, + make_escalation_chain, + make_escalation_policy, + make_alert_group, +): + organization, user = make_organization_and_user() + alert_receive_channel = make_alert_receive_channel(organization) + escalation_chain = make_escalation_chain(organization) + channel_filter = make_channel_filter( + alert_receive_channel, + escalation_chain=escalation_chain, + notification_backends={"BACKEND": {"channel_id": "abc123"}}, + ) + + notify_users_queue = make_escalation_policy( + escalation_chain=channel_filter.escalation_chain, + escalation_policy_step=EscalationPolicy.STEP_NOTIFY_USERS_QUEUE, + last_notified_user=user, + ) + notify_users_queue.notify_to_users_queue.set([user]) + + alert_group = make_alert_group(alert_receive_channel, channel_filter=channel_filter) + alert_group.raw_escalation_snapshot = alert_group.build_raw_escalation_snapshot() + alert_group.save() + escalation_snapshot = alert_group.escalation_snapshot + + assert notify_users_queue.last_notified_user == user + assert escalation_snapshot.escalation_policies_snapshots[0].last_notified_user == user + assert len(escalation_snapshot.escalation_policies_snapshots[0].notify_to_users_queue) == 1 + + # delete user + user.is_active = None + user.save() + + alert_group.raw_escalation_snapshot = alert_group.build_raw_escalation_snapshot() + # clear cached_property + del alert_group.escalation_snapshot + alert_group.save() + + escalation_snapshot = alert_group.escalation_snapshot + + assert notify_users_queue.last_notified_user == user + assert escalation_snapshot is not None + assert escalation_snapshot.escalation_policies_snapshots[0].last_notified_user is None + assert len(escalation_snapshot.escalation_policies_snapshots[0].notify_to_users_queue) == 0 diff --git a/engine/apps/api/tests/test_escalation_policy.py b/engine/apps/api/tests/test_escalation_policy.py index bbb75329..f14d4daa 100644 --- a/engine/apps/api/tests/test_escalation_policy.py +++ b/engine/apps/api/tests/test_escalation_policy.py @@ -846,6 +846,9 @@ def test_escalation_policy_filter_by_user( assert response.status_code == status.HTTP_200_OK + result = response.json() + assert set(result[1]["notify_to_users_queue"]) == {user.public_primary_key, second_user.public_primary_key} + expected_payload[1]["notify_to_users_queue"] = result[1]["notify_to_users_queue"] assert response.json() == expected_payload From 4a53df0c9b89ca4766a222e0faac5b08020ec501 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 4 Sep 2023 11:47:17 +0000 Subject: [PATCH 14/16] Update `make docs` procedure (#2952) Co-authored-by: grafanabot Co-authored-by: Jack Baldry --- docs/make-docs | 52 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/docs/make-docs b/docs/make-docs index 79219618..60759480 100755 --- a/docs/make-docs +++ b/docs/make-docs @@ -6,6 +6,13 @@ # [Semantic versioning](https://semver.org/) is used to help the reader identify the significance of changes. # Changes are relevant to this script and the support docs.mk GNU Make interface. +# ## 4.2.0 (2023-09-01) + +# ### Added + +# - Retry the initial webserver request up to ten times to allow for the process to start. +# If it is still failing after ten seconds, an error message is logged. + # ## 4.1.1 (2023-07-20) # ### Fixed @@ -439,30 +446,39 @@ await_build() { url="$1" req="$(if command -v curl >/dev/null 2>&1; then echo 'curl -s -o /dev/null'; else echo 'wget -q'; fi)" - sleep 2 + i=1 + max=10 + while [ "${i}" -ne "${max}" ] + do + sleep 1 + debg "Retrying request to webserver assuming the process is still starting up." + i=$((i + 1)) - if ${req} "${url}"; then - echo - echo "View documentation locally:" - for x in ${url_src_dst_vers}; do - IFS='^' read -r url _ _ < Date: Mon, 4 Sep 2023 14:07:38 +0200 Subject: [PATCH 15/16] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a648c22..71d583de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.31 (2023-09-04) + ### Fixed - Fix for Cloud plugin install not refreshing page after completion ([2974](https://github.com/grafana/oncall/issues/2874)) From 38a3326c5602f753c54077e2a9b4b015efd5a177 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 4 Sep 2023 09:15:13 -0300 Subject: [PATCH 16/16] Remove IRM plan references from plugin (#2848) Co-authored-by: Joey Orlando --- .../src/containers/IRMBanner/IRMBanner.tsx | 54 ------------------- .../alert_receive_channel.types.ts | 2 - .../src/models/alertgroup/alertgroup.ts | 11 +--- .../src/models/alertgroup/alertgroup.types.ts | 17 ------ grafana-plugin/src/navbar/Header/Header.tsx | 6 --- .../src/pages/incidents/Incidents.tsx | 4 -- .../src/state/rootBaseStore/index.ts | 1 - 7 files changed, 1 insertion(+), 94 deletions(-) delete mode 100644 grafana-plugin/src/containers/IRMBanner/IRMBanner.tsx diff --git a/grafana-plugin/src/containers/IRMBanner/IRMBanner.tsx b/grafana-plugin/src/containers/IRMBanner/IRMBanner.tsx deleted file mode 100644 index fe62f699..00000000 --- a/grafana-plugin/src/containers/IRMBanner/IRMBanner.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import React, { useEffect } from 'react'; - -import { Alert, AlertVariant, Button, HorizontalGroup } from '@grafana/ui'; -import { observer } from 'mobx-react'; - -import Text from 'components/Text/Text'; -import { IRMPlanStatus } from 'models/alertgroup/alertgroup.types'; -import { useStore } from 'state/useStore'; - -const IRMBanner: React.FC = observer(() => { - const store = useStore(); - const { - alertGroupStore, - alertGroupStore: { irmPlan }, - } = store; - - useEffect(() => { - alertGroupStore.fetchIRMPlan(); - }, []); - - if (store.isOpenSource() || !irmPlan?.limits) { - return null; - } - if (irmPlan.limits.isIrmPro || irmPlan.limits.status === IRMPlanStatus.WithinLimits) { - return null; - } - - const statusSeverity: { [key: string]: AlertVariant } = { - [IRMPlanStatus.WithinLimits]: 'success', - [IRMPlanStatus.NearLimit]: 'warning', - [IRMPlanStatus.AtLimit]: 'error', - }; - - return ( - - -
    - - - - ) as any - } - severity={statusSeverity[irmPlan.limits.status]} - buttonContent={undefined} - /> - ); -}); - -export default IRMBanner; diff --git a/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.types.ts b/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.types.ts index f85152f9..37114e3d 100644 --- a/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.types.ts +++ b/grafana-plugin/src/models/alert_receive_channel/alert_receive_channel.types.ts @@ -1,4 +1,3 @@ -import { IRMPlanStatus } from 'models/alertgroup/alertgroup.types'; import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; import { Heartbeat } from 'models/heartbeat/heartbeat.types'; import { User } from 'models/user/user.types'; @@ -31,7 +30,6 @@ export interface AlertReceiveChannel { author: User['pk']; team: GrafanaTeam['id']; created_at: string; - status: IRMPlanStatus; integration_url: string; inbound_email: string; allow_source_based_resolving: boolean; diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index 82ebae8f..5b978afb 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -11,7 +11,7 @@ import { SelectOption } from 'state/types'; import { openErrorNotification, refreshPageError, showApiError } from 'utils'; import LocationHelper from 'utils/LocationHelper'; -import { Alert, AlertAction, IncidentStatus, ResponseIRMPlan } from './alertgroup.types'; +import { Alert, AlertAction, IncidentStatus } from './alertgroup.types'; export class AlertGroupStore extends BaseStore { @observable.shallow @@ -70,9 +70,6 @@ export class AlertGroupStore extends BaseStore { @observable liveUpdatesPaused = false; - @observable - irmPlan: ResponseIRMPlan = undefined; - constructor(rootStore: RootStore) { super(rootStore); @@ -219,12 +216,6 @@ export class AlertGroupStore extends BaseStore { }); } - async fetchIRMPlan() { - if (!this.rootStore.isOpenSource()) { - this.irmPlan = await makeRequest(`/usage-limits`, { method: 'GET' }); - } - } - // methods were moved from rootBaseStore. // TODO check if methods are dublicating existing ones @action diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts index cdb75b3e..5a87160e 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.types.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.types.ts @@ -90,23 +90,6 @@ export interface Alert { has_pormortem?: boolean; // not implemented yet } -export enum IRMPlanStatus { - WithinLimits = 'within-limits', - NearLimit = 'near-limit', - AtLimit = 'at-limit', -} - -export interface ResponseIRMPlan { - limits: { - id: string; - irmProductStartDate: null; - isIrmPro: boolean; - status: IRMPlanStatus; - reasonHTML: string; - upgradeURL: string; - }; -} - interface RenderForWeb { message: any; title: any; diff --git a/grafana-plugin/src/navbar/Header/Header.tsx b/grafana-plugin/src/navbar/Header/Header.tsx index f58f5447..00cae379 100644 --- a/grafana-plugin/src/navbar/Header/Header.tsx +++ b/grafana-plugin/src/navbar/Header/Header.tsx @@ -6,9 +6,7 @@ import { observer } from 'mobx-react'; import gitHubStarSVG from 'assets/img/github_star.svg'; import logo from 'assets/img/logo.svg'; -import Tag from 'components/Tag/Tag'; import Alerts from 'containers/Alerts/Alerts'; -import IRMBanner from 'containers/IRMBanner/IRMBanner'; import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { useStore } from 'state/useStore'; import { APP_SUBTITLE } from 'utils/consts'; @@ -58,13 +56,10 @@ const Header = observer(() => { ); } - const { irmPlan } = store.alertGroupStore; - return ( <>

    Grafana OnCall

    - {irmPlan?.limits && {irmPlan.limits.isIrmPro ? 'IRM Pro' : 'IRM Lite'}}
    {APP_SUBTITLE}
    @@ -76,7 +71,6 @@ const Banners: React.FC = () => { return (
    -
    ); }; diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index 8148d9b6..d7be6d98 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -98,10 +98,6 @@ class Incidents extends React.Component private pollingIntervalId: NodeJS.Timer = undefined; - async componentDidMount() { - await this.props.store.alertGroupStore.fetchIRMPlan(); - } - componentWillUnmount(): void { this.clearPollingInterval(); } diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index 264cdca2..94b0edb8 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -133,7 +133,6 @@ export class RootBaseStore { this.escalationPolicyStore.updateWebEscalationPolicyOptions(), this.escalationPolicyStore.updateEscalationPolicyOptions(), this.escalationPolicyStore.updateNumMinutesInWindowOptions(), - this.alertGroupStore.fetchIRMPlan(), ]); }