From a85a2f6bec1c67e0d5c567b71c7525453c9efa8b Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Tue, 12 Dec 2023 06:30:48 +0800 Subject: [PATCH 1/7] Add more docker image cache sources (#3373) # 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) --------- Co-authored-by: Joey Orlando --- Tiltfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tiltfile b/Tiltfile index ec6e80ba..2b8a79f9 100644 --- a/Tiltfile +++ b/Tiltfile @@ -35,7 +35,7 @@ allow_k8s_contexts(["kind-kind"]) docker_build_sub( "localhost:63628/oncall/engine:dev", context="./engine", - cache_from=["grafana/oncall:latest"], + cache_from=["grafana/oncall:latest", "grafana/oncall:dev"], ignore=["./grafana-plugin/test-results/", "./grafana-plugin/dist/", "./grafana-plugin/e2e-tests/"], child_context=".", target="dev", From c1b82e341874eea49add0ca0c9c560014c1e7f54 Mon Sep 17 00:00:00 2001 From: Dominik Broj Date: Tue, 12 Dec 2023 09:19:09 +0100 Subject: [PATCH 2/7] fix typo and edit template btn position (#3545) ## 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) --- .../src/containers/IntegrationForm/IntegrationForm.tsx | 2 +- .../containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/grafana-plugin/src/containers/IntegrationForm/IntegrationForm.tsx b/grafana-plugin/src/containers/IntegrationForm/IntegrationForm.tsx index cd3c62bf..dd9f9d8a 100644 --- a/grafana-plugin/src/containers/IntegrationForm/IntegrationForm.tsx +++ b/grafana-plugin/src/containers/IntegrationForm/IntegrationForm.tsx @@ -153,7 +153,7 @@ const IntegrationForm = observer((props: IntegrationFormProps) => { {id === 'new' ? ( 'Alert group labeling' ) : ( - navigateToAlertGroupLabels(id)}>Alert group labels + navigateToAlertGroupLabels(id)}>Alert group labeling )}{' '} drawer. diff --git a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx index a83c912b..ec9024cd 100644 --- a/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx +++ b/grafana-plugin/src/containers/IntegrationLabelsForm/IntegrationLabelsForm.tsx @@ -156,7 +156,7 @@ const IntegrationLabelsForm = observer((props: IntegrationLabelsFormProps) => { - + Allows for the extraction and modification of multiple labels from the alert payload using a single template. Supports not only dynamic values but also dynamic keys. The Jinja template must result in From 2c67df86638152b80af7db467bbd146721244388 Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Tue, 12 Dec 2023 13:16:07 +0200 Subject: [PATCH 3/7] Unifiy logo with IRM (#3552) # What this PR does https://github.com/grafana/oncall/issues/1905 --- .../e2e-tests/integrations/heartbeat.test.ts | 3 +++ grafana-plugin/e2e-tests/utils/schedule.ts | 7 ------- grafana-plugin/package.json | 2 +- grafana-plugin/src/PluginPage.tsx | 8 +++++++- .../ColumnsSelectorWrapper/ColumnsModal.tsx | 8 ++++++-- .../src/navbar/Header/Header.module.scss | 19 +++++++++++++++++-- grafana-plugin/src/navbar/Header/Header.tsx | 5 +++-- .../integrations/Integrations.module.scss | 4 ++++ grafana-plugin/yarn.lock | 8 ++++---- 9 files changed, 45 insertions(+), 19 deletions(-) diff --git a/grafana-plugin/e2e-tests/integrations/heartbeat.test.ts b/grafana-plugin/e2e-tests/integrations/heartbeat.test.ts index 374ca6ae..2ba7e16e 100644 --- a/grafana-plugin/e2e-tests/integrations/heartbeat.test.ts +++ b/grafana-plugin/e2e-tests/integrations/heartbeat.test.ts @@ -7,6 +7,7 @@ const HEARTBEAT_SETTINGS_FORM_TEST_ID = 'heartbeat-settings-form'; test.describe("updating an integration's heartbeat interval works", async () => { const _openHeartbeatSettingsForm = async (page: Page) => { await page.getByTestId('integration-settings-context-menu-wrapper').getByRole('img').click(); + await page.waitForTimeout(1000); await page.getByTestId('integration-heartbeat-settings').click(); }; @@ -29,6 +30,8 @@ test.describe("updating an integration's heartbeat interval works", async () => await heartbeatSettingsForm.getByTestId('update-heartbeat').click(); + await page.waitForTimeout(1000); + await _openHeartbeatSettingsForm(page); const heartbeatIntervalValue = await heartbeatSettingsForm diff --git a/grafana-plugin/e2e-tests/utils/schedule.ts b/grafana-plugin/e2e-tests/utils/schedule.ts index 3627109a..9c5257f6 100644 --- a/grafana-plugin/e2e-tests/utils/schedule.ts +++ b/grafana-plugin/e2e-tests/utils/schedule.ts @@ -20,13 +20,6 @@ export const createOnCallSchedule = async (page: Page, scheduleName: string, use await clickButton({ page, buttonText: 'Add rotation' }); - /** - * Drag the modal such that the "Create" button will always be visible within the viewport. We cannot scroll - * on the modal itself - * https://playwright.dev/docs/input#dragging-manually - */ - await page.locator('.ReactModal__Content .drag-handler').dragTo(page.locator('.page-header__logo')); - await selectDropdownValue({ page, selectType: 'grafanaSelect', diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 60ac20c9..719b6c57 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -122,7 +122,7 @@ "@grafana/data": "^9.2.4", "@grafana/faro-web-sdk": "^1.0.0-beta4", "@grafana/faro-web-tracing": "^1.0.0-beta4", - "@grafana/labels": "~1.4.2", + "@grafana/labels": "~1.4.4", "@grafana/runtime": "9.3.0-beta1", "@grafana/ui": "^10.2.0", "@lifeomic/attempt": "^3.0.3", diff --git a/grafana-plugin/src/PluginPage.tsx b/grafana-plugin/src/PluginPage.tsx index 4497af92..5b81bdc2 100644 --- a/grafana-plugin/src/PluginPage.tsx +++ b/grafana-plugin/src/PluginPage.tsx @@ -3,8 +3,10 @@ import React from 'react'; import { PluginPageProps, PluginPage as RealPluginPage } from '@grafana/runtime'; import Header from 'navbar/Header/Header'; +import RenderConditionally from 'components/RenderConditionally/RenderConditionally'; import { pages } from 'pages'; import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; +import { DEFAULT_PAGE } from 'utils/consts'; interface AppPluginPageProps extends PluginPageProps { page?: string; @@ -14,10 +16,14 @@ export const PluginPage = (isTopNavbar() ? RealPlugin : PluginPageFallback) as R function RealPlugin(props: AppPluginPageProps): React.ReactNode { const { page } = props; + const isDefaultPage = page === DEFAULT_PAGE; return ( -
+ +
+ + {pages[page]?.text && !pages[page]?.hideTitle && (

{pages[page].text} diff --git a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx index b4a85352..5150caac 100644 --- a/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx +++ b/grafana-plugin/src/containers/ColumnsSelectorWrapper/ColumnsModal.tsx @@ -12,8 +12,10 @@ import { VerticalGroup, useStyles2, } from '@grafana/ui'; +import cn from 'classnames/bind'; import { observer } from 'mobx-react'; +import styles from 'assets/style/utils.css'; import Block from 'components/GBlock/Block'; import Text from 'components/Text/Text'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; @@ -28,6 +30,8 @@ import { useDebouncedCallback } from 'utils/hooks'; import { getColumnsSelectorWrapperStyles } from './ColumnsSelectorWrapper.styles'; +const cx = cn.bind(styles); + interface ColumnsModalProps { isModalOpen: boolean; labelKeys: Array; @@ -108,7 +112,7 @@ export const ColumnsModal: React.FC = observer( {!result.isCollapsed && ( {result.values === undefined ? ( - + ) : ( renderLabelValues(result.name, result.values) )} @@ -135,7 +139,7 @@ export const ColumnsModal: React.FC = observer( variant="primary" onClick={onAddNewColumns} > - {isLoading ? : 'Add'} + {isLoading ? : 'Add'} diff --git a/grafana-plugin/src/navbar/Header/Header.module.scss b/grafana-plugin/src/navbar/Header/Header.module.scss index 78318cca..efa4bcd3 100644 --- a/grafana-plugin/src/navbar/Header/Header.module.scss +++ b/grafana-plugin/src/navbar/Header/Header.module.scss @@ -4,7 +4,8 @@ .header-topnavbar { padding-top: 0; - padding-bottom: 36px; + padding-bottom: 0; + margin-bottom: 32px; } .navbar-heading { @@ -25,6 +26,7 @@ .navbar-left { display: flex; flex-basis: 100%; + align-items: flex-start; } .navbar-heading-container { @@ -34,6 +36,7 @@ flex-direction: row; column-gap: 8px; row-gap: 8px; + margin-left: -50px; } .irm-icon { @@ -44,7 +47,6 @@ } .banners { - padding-top: 12px; margin-bottom: 24px; &:empty { @@ -52,3 +54,16 @@ margin-bottom: 0; } } + +.logo-container, +.page-header__img { + height: 32px; +} + +.logo-container { + margin-top: 2px; +} + +.page-header__title { + margin-bottom: 8px; +} diff --git a/grafana-plugin/src/navbar/Header/Header.tsx b/grafana-plugin/src/navbar/Header/Header.tsx index 00cae379..c7878ae5 100644 --- a/grafana-plugin/src/navbar/Header/Header.tsx +++ b/grafana-plugin/src/navbar/Header/Header.tsx @@ -23,8 +23,8 @@ const Header = observer(() => {
- - Grafana OnCall + + Grafana OnCall
{renderHeading()}
@@ -41,6 +41,7 @@ const Header = observer(() => {

Grafana OnCall

{APP_SUBTITLE}
+ Date: Tue, 12 Dec 2023 13:01:47 +0100 Subject: [PATCH 4/7] Fix task retries for deleted alert groups (#3553) # What this PR does ## Which issue(s) this PR fixes ## 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) --- .../apps/alerts/tasks/send_update_log_report_signal.py | 2 +- engine/apps/telegram/tasks.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/engine/apps/alerts/tasks/send_update_log_report_signal.py b/engine/apps/alerts/tasks/send_update_log_report_signal.py index 8d40efab..a59ad75c 100644 --- a/engine/apps/alerts/tasks/send_update_log_report_signal.py +++ b/engine/apps/alerts/tasks/send_update_log_report_signal.py @@ -7,7 +7,7 @@ from .task_logger import task_logger @shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None + autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else 10 ) def send_update_log_report_signal(log_record_pk=None, alert_group_pk=None): from apps.alerts.models import AlertGroup, AlertReceiveChannel diff --git a/engine/apps/telegram/tasks.py b/engine/apps/telegram/tasks.py index f2fbabb0..d6fa1bec 100644 --- a/engine/apps/telegram/tasks.py +++ b/engine/apps/telegram/tasks.py @@ -184,8 +184,14 @@ def on_create_alert_telegram_representative_async(self, alert_pk): """ It's async in order to prevent Telegram downtime or formatting issues causing delay with SMS and other destinations. """ - - alert = Alert.objects.get(pk=alert_pk) + try: + alert = Alert.objects.get(pk=alert_pk) + except Alert.DoesNotExist as e: + if on_create_alert_telegram_representative_async.request.retries >= 10: + logger.error(f"Alert {alert_pk} was not found. Probably it was deleted. Stop retrying") + return + else: + raise e alert_group = alert.group alert_group_messages = alert_group.telegram_messages.filter( From 0861113ed5774cd4dea97ca626dd6e119fb03777 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Tue, 12 Dec 2023 13:02:26 +0100 Subject: [PATCH 5/7] Add error code for mobile notification logs (#3554) # What this PR does Adds error code for mobile notification logs ## 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) --- CHANGELOG.md | 4 ++++ .../base/models/user_notification_policy_log_record.py | 8 +++++++- engine/apps/mobile_app/tasks/new_alert_group.py | 5 +++-- .../apps/mobile_app/tests/tasks/test_new_alert_group.py | 4 ++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 826bcc0f..915fa4c7 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 +### Changed + +- Add error code for mobile push notification logs when device is not set up @Ferril ([#3554](https://github.com/grafana/oncall/pull/3554)) + ## v1.3.77 (2023-12-11) ### Fixed diff --git a/engine/apps/base/models/user_notification_policy_log_record.py b/engine/apps/base/models/user_notification_policy_log_record.py index 1c28a092..d8c83e31 100644 --- a/engine/apps/base/models/user_notification_policy_log_record.py +++ b/engine/apps/base/models/user_notification_policy_log_record.py @@ -69,7 +69,8 @@ class UserNotificationPolicyLogRecord(models.Model): ERROR_NOTIFICATION_MESSAGING_BACKEND_ERROR, ERROR_NOTIFICATION_FORBIDDEN, ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED, - ) = range(27) + ERROR_NOTIFICATION_MOBILE_USER_HAS_NO_ACTIVE_DEVICE, + ) = range(28) # for this errors we want to send message to general log channel ERRORS_TO_SEND_IN_SLACK_CHANNEL = [ @@ -264,6 +265,11 @@ class UserNotificationPolicyLogRecord(models.Model): == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED ): result += f"failed to send telegram message to {user_verbal} because user has been deactivated" + elif ( + self.notification_error_code + == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_MOBILE_USER_HAS_NO_ACTIVE_DEVICE + ): + result += f"failed to send push notification to {user_verbal} because user has no device set up" else: # TODO: handle specific backend errors try: diff --git a/engine/apps/mobile_app/tasks/new_alert_group.py b/engine/apps/mobile_app/tasks/new_alert_group.py index 715d6300..e359328b 100644 --- a/engine/apps/mobile_app/tasks/new_alert_group.py +++ b/engine/apps/mobile_app/tasks/new_alert_group.py @@ -113,7 +113,7 @@ def notify_user_about_new_alert_group(user_pk, alert_group_pk, notification_poli logger.warning(f"User notification policy {notification_policy_pk} does not exist") return - def _create_error_log_record(): + def _create_error_log_record(notification_error_code=None): """ Utility method to create a UserNotificationPolicyLogRecord with error """ @@ -125,13 +125,14 @@ def notify_user_about_new_alert_group(user_pk, alert_group_pk, notification_poli reason="Mobile push notification error", notification_step=notification_policy.step, notification_channel=notification_policy.notify_by, + notification_error_code=notification_error_code, ) device_to_notify = FCMDevice.get_active_device_for_user(user) # create an error log in case user has no devices set up if not device_to_notify: - _create_error_log_record() + _create_error_log_record(UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_MOBILE_USER_HAS_NO_ACTIVE_DEVICE) logger.error(f"Error while sending a mobile push notification: user {user_pk} has no device set up") return diff --git a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py index 5c784497..1800753f 100644 --- a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py +++ b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py @@ -82,6 +82,10 @@ def test_notify_user_about_new_alert_group_no_device_connected( log_record = alert_group.personal_log_records.last() assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED + assert ( + log_record.notification_error_code + == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_MOBILE_USER_HAS_NO_ACTIVE_DEVICE + ) @pytest.mark.django_db From e003e8a0b86cdb96d7c6fb682b2bdfcb26157870 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Tue, 12 Dec 2023 17:46:08 +0100 Subject: [PATCH 6/7] Fix `message is too big` exception for mobile push notification (#3556) # What this PR does Adds limit for alert title length in mobile app push notifications ## Which issue(s) this PR fixes https://github.com/grafana/oncall-private/issues/2375 ## 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 +++ engine/apps/mobile_app/alert_rendering.py | 5 +++ .../tests/tasks/test_new_alert_group.py | 36 +++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 915fa4c7..5c17a650 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add error code for mobile push notification logs when device is not set up @Ferril ([#3554](https://github.com/grafana/oncall/pull/3554)) +### Fixed + +- Fix issue when mobile push notification message is too big @Ferril ([#3556](https://github.com/grafana/oncall/pull/3556) + ## v1.3.77 (2023-12-11) ### Fixed diff --git a/engine/apps/mobile_app/alert_rendering.py b/engine/apps/mobile_app/alert_rendering.py index ef5082db..37453c15 100644 --- a/engine/apps/mobile_app/alert_rendering.py +++ b/engine/apps/mobile_app/alert_rendering.py @@ -10,9 +10,14 @@ class AlertMobileAppTemplater(AlertTemplater): def get_push_notification_subtitle(alert_group): + MAX_ALERT_TITLE_LENGTH = 200 alert = alert_group.alerts.first() templated_alert = AlertMobileAppTemplater(alert).render() alert_title = str_or_backup(templated_alert.title, "Alert Group") + # limit alert title length to prevent FCM `message is too big` exception + # https://firebase.google.com/docs/cloud-messaging/concept-options#notifications_and_data_messages + if len(alert_title) > MAX_ALERT_TITLE_LENGTH: + alert_title = f"{alert_title[:MAX_ALERT_TITLE_LENGTH]}..." status_verbose = "Firing" # TODO: we should probably de-duplicate this text if alert_group.resolved: diff --git a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py index 1800753f..fec871e1 100644 --- a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py +++ b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py @@ -2,7 +2,9 @@ from unittest.mock import patch import pytest +from apps.alerts.incident_appearance.templaters.alert_templater import TemplatedAlert from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord +from apps.mobile_app.alert_rendering import AlertMobileAppTemplater, get_push_notification_subtitle from apps.mobile_app.models import FCMDevice, MobileAppUserSettings from apps.mobile_app.tasks.new_alert_group import _get_fcm_message, notify_user_about_new_alert_group @@ -175,3 +177,37 @@ def test_fcm_message_user_settings_critical_override_dnd_disabled( apns_sound = message.apns.payload.aps.sound assert apns_sound.critical is False assert message.apns.payload.aps.custom_data["interruption-level"] == "time-sensitive" + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "alert_title", + [ + "Some short title", + "Some long title" * 100, + ], +) +def test_get_push_notification_subtitle( + alert_title, + make_organization_and_user, + make_alert_receive_channel, + make_alert_group, + make_alert, +): + MAX_ALERT_TITLE_LENGTH = 200 + organization, user = make_organization_and_user() + alert_receive_channel = make_alert_receive_channel(organization=organization) + alert_group = make_alert_group(alert_receive_channel) + make_alert(alert_group=alert_group, title=alert_title, raw_request_data={"title": alert_title}) + expected_alert_title = ( + f"{alert_title[:MAX_ALERT_TITLE_LENGTH]}..." if len(alert_title) > MAX_ALERT_TITLE_LENGTH else alert_title + ) + expected_result = ( + f"#1 {expected_alert_title}\n" + f"via {alert_group.channel.short_name}" + "\nStatus: Firing, alerts: 1" + ) + templated_alert = TemplatedAlert() + templated_alert.title = alert_title + with patch.object(AlertMobileAppTemplater, "render", return_value=templated_alert): + result = get_push_notification_subtitle(alert_group) + assert len(expected_alert_title) <= MAX_ALERT_TITLE_LENGTH + 3 + assert result == expected_result From d6873b0d73c72e80744fadfd04ab4b391a937f18 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 12 Dec 2023 13:46:32 -0700 Subject: [PATCH 7/7] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c17a650..61de9a7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## v1.3.78 (2023-12-12) ### Changed