From bc6272744b1ddee4518d4a27dd3b7ef8d921be16 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 21 Jun 2024 14:40:34 +0800 Subject: [PATCH 01/19] Post stack slug to chatops proxy (#4559) # What this PR does It's part of work on https://github.com/grafana/oncall-gateway/issues/247. I added stack_slug to call to the chatops-proxy tenant/register API. On a side note I figured out that we didn't cleanup slack integration in chatops-proxy, once it's uninstalled on OnCall side, so it's [fixed](https://github.com/grafana/oncall/pull/4559/files#diff-1784f1d0d65fa477f4562e73aa23fe1c757b171f36e03f12600bdb021f121307R577) as well. Changes are validated locally. --- engine/apps/chatops_proxy/client.py | 19 ++++++++++++++- engine/apps/chatops_proxy/tasks.py | 4 +++- engine/apps/chatops_proxy/utils.py | 23 ++++++++++++++++++- engine/apps/slack/views.py | 12 ++++++++++ .../user_management/models/organization.py | 4 +++- 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/engine/apps/chatops_proxy/client.py b/engine/apps/chatops_proxy/client.py index baa382d5b4..1a0b63bcb1 100644 --- a/engine/apps/chatops_proxy/client.py +++ b/engine/apps/chatops_proxy/client.py @@ -65,7 +65,7 @@ def __init__(self, url: str, token: str): # OnCall Tenant def register_tenant( - self, service_tenant_id: str, cluster_slug: str, service_type: str, stack_id: int + self, service_tenant_id: str, cluster_slug: str, service_type: str, stack_id: int, stack_slug: str ) -> tuple[Tenant, requests.models.Response]: url = f"{self.api_base_url}/tenants/register" d = { @@ -74,6 +74,7 @@ def register_tenant( "cluster_slug": cluster_slug, "service_type": service_type, "stack_id": stack_id, + "stack_slug": stack_slug, } } response = requests.post(url=url, json=d, headers=self._headers) @@ -170,6 +171,22 @@ def get_oauth_installation( self._check_response(response) return OAuthInstallation(**response.json()["oauth_installation"]), response + def delete_oauth_installation( + self, + stack_id: int, + provider_type: str, + grafana_user_id: int, + ) -> tuple[bool, requests.models.Response]: + url = f"{self.api_base_url}/oauth_installations/uninstall" + d = { + "stack_id": stack_id, + "provider_type": provider_type, + "grafana_user_id": grafana_user_id, + } + response = requests.post(url=url, json=d, headers=self._headers) + self._check_response(response) + return response.json()["removed"], response + def _check_response(self, response: requests.models.Response): """ Wraps an exceptional response to ChatopsProxyAPIException diff --git a/engine/apps/chatops_proxy/tasks.py b/engine/apps/chatops_proxy/tasks.py index bbe3dcd0bc..2793dca5ea 100644 --- a/engine/apps/chatops_proxy/tasks.py +++ b/engine/apps/chatops_proxy/tasks.py @@ -18,14 +18,16 @@ def register_oncall_tenant_async(**kwargs): cluster_slug = kwargs.get("cluster_slug") service_type = kwargs.get("service_type") stack_id = kwargs.get("stack_id") + stack_slug = kwargs.get("stack_slug") client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) try: - client.register_tenant(service_tenant_id, cluster_slug, service_type, stack_id) + client.register_tenant(service_tenant_id, cluster_slug, service_type, stack_id, stack_slug) except ChatopsProxyAPIException as api_exc: task_logger.error( f'msg="Failed to register OnCall tenant: {api_exc.msg}" service_tenant_id={service_tenant_id} cluster_slug={cluster_slug}' ) + # TODO: remove this check once new upsert tenant api is released if api_exc.status == 409: # 409 Indicates that it's impossible to register tenant, because tenant already registered. # Not retrying in this case, because manual conflict-resolution needed. diff --git a/engine/apps/chatops_proxy/utils.py b/engine/apps/chatops_proxy/utils.py index 60d7039116..6be8ea7d40 100644 --- a/engine/apps/chatops_proxy/utils.py +++ b/engine/apps/chatops_proxy/utils.py @@ -48,7 +48,7 @@ def get_slack_oauth_response_from_chatops_proxy(stack_id) -> dict: return slack_installation.oauth_response -def register_oncall_tenant(service_tenant_id: str, cluster_slug: str, stack_id: int): +def register_oncall_tenant(service_tenant_id: str, cluster_slug: str, stack_id: int, stack_slug: str): """ register_oncall_tenant tries to register oncall tenant synchronously and fall back to task in case of any exceptions to make sure that tenant is registered. @@ -61,6 +61,7 @@ def register_oncall_tenant(service_tenant_id: str, cluster_slug: str, stack_id: cluster_slug, SERVICE_TYPE_ONCALL, stack_id, + stack_slug, ) except Exception as e: logger.error( @@ -141,3 +142,23 @@ def unlink_slack_team(service_tenant_id: str, slack_team_id: str): "service_type": SERVICE_TYPE_ONCALL, } ) + + +def uninstall_slack(stack_id: int, grafana_user_id: int) -> bool: + """ + uninstall_slack uninstalls slack integration from chatops-proxy and returns bool indicating if it was removed. + If such installation does not exist - returns True as well.s + """ + client = ChatopsProxyAPIClient(settings.ONCALL_GATEWAY_URL, settings.ONCALL_GATEWAY_API_TOKEN) + try: + removed, response = client.delete_oauth_installation(stack_id, PROVIDER_TYPE_SLACK, grafana_user_id) + except ChatopsProxyAPIException as api_exc: + if api_exc.status == 404: + return True + logger.exception( + "uninstall_slack: error trying to install slack from chatops-proxy: " "error=%s", + api_exc, + ) + return False + + return removed is True diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index e921c60925..9578bbc43d 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -15,6 +15,7 @@ from apps.api.permissions import RBACPermission from apps.auth_token.auth import PluginAuthentication from apps.base.utils import live_settings +from apps.chatops_proxy.utils import uninstall_slack as uninstall_slack_from_chatops_proxy from apps.slack.client import SlackClient from apps.slack.errors import SlackAPIError from apps.slack.scenarios.alertgroup_appearance import STEPS_ROUTING as ALERTGROUP_APPEARANCE_ROUTING @@ -573,8 +574,19 @@ def post(self, request): "Grafana OnCall is temporary unable to connect your slack account or install OnCall to your slack workspace", status=400, ) + if settings.UNIFIED_SLACK_APP_ENABLED: + # If unified slack app is enabled - uninstall slack integration from chatops-proxy first and on success - + # uninstall it from OnCall. + removed = uninstall_slack_from_chatops_proxy(request.user.organization.stack_id, request.user.user_id) + else: + # just a placeholder value to continute uninstallation until UNIFIED_SLACK_APP_ENABLED is not enabled + removed = True + if not removed: + return Response({"error": "Failed to uninstall slack integration"}, status=500) + try: uninstall_slack_integration(request.user.organization, request.user) except SlackInstallationExc as e: return Response({"error": e.error_message}, status=400) + return Response(status=200) diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index e82b8aceab..506951e2ad 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -61,7 +61,9 @@ class OrganizationQuerySet(models.QuerySet): def create(self, **kwargs): instance = super().create(**kwargs) if settings.FEATURE_MULTIREGION_ENABLED: - register_oncall_tenant(str(instance.uuid), settings.ONCALL_BACKEND_REGION, instance.stack_id) + register_oncall_tenant( + str(instance.uuid), settings.ONCALL_BACKEND_REGION, instance.stack_id, instance.stack_slug + ) return instance def delete(self): From 615081a112f1dcd57c79209daf3e566d8a678233 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Fri, 21 Jun 2024 11:11:23 +0100 Subject: [PATCH 02/19] Inject chatops-proxy metadata into direct paging user dropdown (#4556) Related to https://github.com/grafana/oncall-gateway/issues/240 ## 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] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/slack/scenarios/paging.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index 22532dc0f9..b9191234e1 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -706,6 +706,13 @@ def _create_user_option_groups( } ) + # Only inject chatops-proxy metadata into the first dropdown option to reduce payload size + # so the 250kb Slack limit is not exceeded for orgs with many users + if option_groups: + option_groups[0]["options"][0]["value"] = make_value( + json.loads(option_groups[0]["options"][0]["value"]), organization + ) + return option_groups From 16e98da64aa6fe12a384f7025ef8cbd6f8ccee25 Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Fri, 21 Jun 2024 17:07:46 +0300 Subject: [PATCH 03/19] Tweaks to overrides (added draggable bounds, take timezone into consideration) (#4553) # What this PR does Part of https://github.com/grafana/oncall/issues/4428 ## Which issue(s) this PR closes Closes https://github.com/grafana/oncall/issues/4547 --- .../e2e-tests/alerts/onCallSchedule.test.ts | 4 +- .../e2e-tests/insights/insights.test.ts | 4 +- .../e2e-tests/schedules/addOverride.test.ts | 46 ++++- .../e2e-tests/schedules/addRotation.test.ts | 43 +++++ .../e2e-tests/schedules/quality.test.ts | 4 +- .../schedules/scheduleDetails.test.ts | 4 +- .../e2e-tests/schedules/scheduleView.test.ts | 4 +- .../e2e-tests/schedules/schedulesList.test.ts | 4 +- .../e2e-tests/schedules/timezones.test.ts | 7 +- grafana-plugin/e2e-tests/utils/constants.ts | 2 + .../e2e-tests/utils/grafanaProfile.ts | 5 +- grafana-plugin/e2e-tests/utils/schedule.ts | 9 +- .../RotationForm/RotationForm.helpers.ts | 41 +++++ .../containers/RotationForm/RotationForm.tsx | 76 ++------ .../RotationForm/ScheduleOverrideForm.tsx | 171 +++++++++++------- .../containers/Rotations/Rotations.helpers.ts | 8 + .../src/containers/Rotations/Rotations.tsx | 7 +- .../Rotations/ScheduleOverrides.tsx | 30 +-- .../src/pages/schedule/Schedule.tsx | 8 +- grafana-plugin/src/utils/DOM.ts | 2 +- grafana-plugin/src/utils/hooks.tsx | 10 + 21 files changed, 326 insertions(+), 163 deletions(-) create mode 100644 grafana-plugin/e2e-tests/schedules/addRotation.test.ts diff --git a/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts b/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts index f2e8d0f124..6859ec0378 100644 --- a/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts +++ b/grafana-plugin/e2e-tests/alerts/onCallSchedule.test.ts @@ -3,7 +3,7 @@ import { verifyThatAlertGroupIsTriggered } from '../utils/alertGroup'; import { createEscalationChain, EscalationStep } from '../utils/escalationChain'; import { generateRandomValue } from '../utils/forms'; import { createIntegrationAndSendDemoAlert } from '../utils/integrations'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('we can create an oncall schedule + receive an alert', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; @@ -11,7 +11,7 @@ test('we can create an oncall schedule + receive an alert', async ({ adminRolePa const integrationName = generateRandomValue(); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await createEscalationChain( page, escalationChainName, diff --git a/grafana-plugin/e2e-tests/insights/insights.test.ts b/grafana-plugin/e2e-tests/insights/insights.test.ts index 14d2feeca7..411a8cd4f4 100644 --- a/grafana-plugin/e2e-tests/insights/insights.test.ts +++ b/grafana-plugin/e2e-tests/insights/insights.test.ts @@ -6,7 +6,7 @@ import { createEscalationChain, EscalationStep } from '../utils/escalationChain' import { clickButton, generateRandomValue } from '../utils/forms'; import { createIntegrationAndSendDemoAlert } from '../utils/integrations'; import { goToGrafanaPage, goToOnCallPage } from '../utils/navigation'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; /** * Insights is dependent on Scenes which were only added in Grafana 10.0.0 @@ -66,7 +66,7 @@ test.describe.skip('Insights', () => { const escalationChainName = generateRandomValue(); const integrationName = generateRandomValue(); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await createEscalationChain( page, escalationChainName, diff --git a/grafana-plugin/e2e-tests/schedules/addOverride.test.ts b/grafana-plugin/e2e-tests/schedules/addOverride.test.ts index 709c85511f..a741c9338a 100644 --- a/grafana-plugin/e2e-tests/schedules/addOverride.test.ts +++ b/grafana-plugin/e2e-tests/schedules/addOverride.test.ts @@ -1,14 +1,16 @@ import dayjs from 'dayjs'; -import { test, expect } from '../fixtures'; +import { test, expect, Locator } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; import { clickButton, generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation, getOverrideFormDateInputs } from '../utils/schedule'; +import { setTimezoneInProfile } from '../utils/grafanaProfile'; +import { createOnCallSchedule, getOverrideFormDateInputs } from '../utils/schedule'; -test('default dates in override creation modal are correct', async ({ adminRolePage }) => { +test('Default dates in override creation modal are set to today', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await clickButton({ page, buttonText: 'Add override' }); @@ -20,3 +22,39 @@ test('default dates in override creation modal are correct', async ({ adminRoleP expect(overrideFormDateInputs.start.isSame(expectedStart)).toBe(true); expect(overrideFormDateInputs.end.isSame(expectedEnd)).toBe(true); }); + +test('Fills in override time and reacts to timezone change', async ({ adminRolePage }) => { + const { page, userName } = adminRolePage; + + await setTimezoneInProfile(page, MOSCOW_TIMEZONE); // UTC+3 + + const onCallScheduleName = generateRandomValue(); + await createOnCallSchedule(page, onCallScheduleName, userName, false); + + await clickButton({ page, buttonText: 'Add override' }); + + const overrideStartEl = page.getByTestId('override-start'); + await changeDatePickerTime(overrideStartEl, '02'); + await expect(overrideStartEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('02:00'); + + const overrideEndEl = page.getByTestId('override-end'); + await changeDatePickerTime(overrideEndEl, '12'); + await expect(overrideEndEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('12:00'); + + await page.getByRole('dialog').click(); // clear focus + + await page.getByTestId('timezone-select').locator('svg').click(); + await page.getByText('GMT', { exact: true }).click(); + + // expect times to go back by -3 + await expect(overrideStartEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('23:00'); + await expect(overrideEndEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('09:00'); + + async function changeDatePickerTime(element: Locator, value: string) { + await element.getByRole('img').click(); + // set minutes to {value} + await page.locator('.rc-time-picker-panel').getByRole('button', { name: value }).first().click(); + // set seconds to 00 + await page.getByRole('button', { name: '00' }).nth(1).click(); + } +}); diff --git a/grafana-plugin/e2e-tests/schedules/addRotation.test.ts b/grafana-plugin/e2e-tests/schedules/addRotation.test.ts new file mode 100644 index 0000000000..478e69033e --- /dev/null +++ b/grafana-plugin/e2e-tests/schedules/addRotation.test.ts @@ -0,0 +1,43 @@ +import { test, expect, Locator } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; +import { clickButton, generateRandomValue } from '../utils/forms'; +import { setTimezoneInProfile } from '../utils/grafanaProfile'; +import { createOnCallSchedule } from '../utils/schedule'; + +test('Fills in Rotation time and reacts to timezone change', async ({ adminRolePage }) => { + const { page, userName } = adminRolePage; + + await setTimezoneInProfile(page, MOSCOW_TIMEZONE); // UTC+3 + + const onCallScheduleName = generateRandomValue(); + await createOnCallSchedule(page, onCallScheduleName, userName, false); + + await clickButton({ page, buttonText: 'Add rotation' }); + // enable Rotation End + await page.getByTestId('rotation-end').getByLabel('Toggle switch').click(); + + const startEl = page.getByTestId('rotation-start'); + await changeDatePickerTime(startEl, '02'); + await expect(startEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('02:00'); + + const endEl = page.getByTestId('rotation-end'); + await changeDatePickerTime(endEl, '12'); + await expect(endEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('12:00'); + + await page.getByRole('dialog').click(); // clear focus + + await page.getByTestId('timezone-select').locator('svg').click(); + await page.getByText('GMT', { exact: true }).click(); + + // expect times to go back by -3 + await expect(startEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('23:00'); + await expect(endEl.getByTestId('date-time-picker').getByRole('textbox')).toHaveValue('09:00'); + + async function changeDatePickerTime(element: Locator, value: string) { + await element.getByRole('img').click(); + // set minutes to {value} + await page.locator('.rc-time-picker-panel').getByRole('button', { name: value }).first().click(); + // set seconds to 00 + await page.getByRole('button', { name: '00' }).nth(1).click(); + } +}); diff --git a/grafana-plugin/e2e-tests/schedules/quality.test.ts b/grafana-plugin/e2e-tests/schedules/quality.test.ts index abd26d9c3c..82d84212c1 100644 --- a/grafana-plugin/e2e-tests/schedules/quality.test.ts +++ b/grafana-plugin/e2e-tests/schedules/quality.test.ts @@ -1,12 +1,12 @@ import { test, expect } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('check schedule quality for simple 1-user schedule', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); const scheduleQualityElement = page.getByTestId('schedule-quality'); await scheduleQualityElement.waitFor({ state: 'visible' }); diff --git a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts index e1bd7f3295..e092bb610a 100644 --- a/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts +++ b/grafana-plugin/e2e-tests/schedules/scheduleDetails.test.ts @@ -1,13 +1,13 @@ import { test, expect } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation, createRotation } from '../utils/schedule'; +import { createOnCallSchedule, createRotation } from '../utils/schedule'; test(`user can see the other user's details`, async ({ adminRolePage, editorRolePage }) => { const { page, userName: adminUserName } = adminRolePage; const editorUserName = editorRolePage.userName; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, adminUserName); + await createOnCallSchedule(page, onCallScheduleName, adminUserName); await createRotation(page, editorUserName, false); await page.waitForTimeout(1_000); diff --git a/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts b/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts index 1def16456e..0f27f47ac1 100644 --- a/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts +++ b/grafana-plugin/e2e-tests/schedules/scheduleView.test.ts @@ -4,13 +4,13 @@ import { HTML_ID } from 'utils/DOM'; import { expect, test } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test.skip('schedule view (week/2 weeks/month) toggler works', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); // ScheduleView.OneWeek is selected by default expect(await page.getByLabel(ScheduleView.OneWeek, { exact: true }).isChecked()).toBe(true); diff --git a/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts b/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts index d83b570ebd..31c50c1fcf 100644 --- a/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts +++ b/grafana-plugin/e2e-tests/schedules/schedulesList.test.ts @@ -1,13 +1,13 @@ import { expect, test } from '../fixtures'; import { generateRandomValue } from '../utils/forms'; import { goToOnCallPage } from '../utils/navigation'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; test('schedule calendar and list of schedules is correctly displayed', async ({ adminRolePage }) => { const { page, userName } = adminRolePage; const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); await goToOnCallPage(page, 'schedules'); diff --git a/grafana-plugin/e2e-tests/schedules/timezones.test.ts b/grafana-plugin/e2e-tests/schedules/timezones.test.ts index 32f5689d6d..71df96f620 100644 --- a/grafana-plugin/e2e-tests/schedules/timezones.test.ts +++ b/grafana-plugin/e2e-tests/schedules/timezones.test.ts @@ -4,15 +4,14 @@ import isoWeek from 'dayjs/plugin/isoWeek'; import utc from 'dayjs/plugin/utc'; import { test } from '../fixtures'; +import { MOSCOW_TIMEZONE } from '../utils/constants'; import { clickButton, generateRandomValue } from '../utils/forms'; import { setTimezoneInProfile } from '../utils/grafanaProfile'; -import { createOnCallScheduleWithRotation } from '../utils/schedule'; +import { createOnCallSchedule } from '../utils/schedule'; dayjs.extend(utc); dayjs.extend(isoWeek); -const MOSCOW_TIMEZONE = 'Europe/Moscow'; - test.use({ timezoneId: MOSCOW_TIMEZONE }); // GMT+3 the whole year const currentUtcTimeHour = dayjs().utc().format('HH'); const currentUtcDate = dayjs().utc().format('DD MMM'); @@ -25,7 +24,7 @@ test('dates in schedule are correct according to selected current timezone', asy await setTimezoneInProfile(page, MOSCOW_TIMEZONE); const onCallScheduleName = generateRandomValue(); - await createOnCallScheduleWithRotation(page, onCallScheduleName, userName); + await createOnCallSchedule(page, onCallScheduleName, userName); // Current timezone is selected by default to currently logged in user timezone await expect(page.getByTestId('timezone-select')).toHaveText('GMT+3'); diff --git a/grafana-plugin/e2e-tests/utils/constants.ts b/grafana-plugin/e2e-tests/utils/constants.ts index f6969efdf7..5345d1c4d7 100644 --- a/grafana-plugin/e2e-tests/utils/constants.ts +++ b/grafana-plugin/e2e-tests/utils/constants.ts @@ -10,3 +10,5 @@ export const GRAFANA_ADMIN_PASSWORD = process.env.GRAFANA_ADMIN_PASSWORD || 'onc export const IS_OPEN_SOURCE = (process.env.IS_OPEN_SOURCE || 'true').toLowerCase() === 'true'; export const IS_CLOUD = !IS_OPEN_SOURCE; + +export const MOSCOW_TIMEZONE = 'Europe/Moscow'; diff --git a/grafana-plugin/e2e-tests/utils/grafanaProfile.ts b/grafana-plugin/e2e-tests/utils/grafanaProfile.ts index 672de8cdb7..add2e32f32 100644 --- a/grafana-plugin/e2e-tests/utils/grafanaProfile.ts +++ b/grafana-plugin/e2e-tests/utils/grafanaProfile.ts @@ -1,11 +1,14 @@ -import { Page } from '@playwright/test'; +import { Page, expect } from '@playwright/test'; import { goToGrafanaPage } from './navigation'; export const setTimezoneInProfile = async (page: Page, timezone: string) => { await goToGrafanaPage(page, '/profile'); + await expect(page.getByLabel('Time zone picker')).toBeVisible(); + await page.getByLabel('Time zone picker').click(); await page.getByLabel('Select options menu').getByText(timezone).click(); await page.getByTestId('data-testid-shared-prefs-save').click(); await page.waitForLoadState('networkidle'); + await page.waitForTimeout(3000); // wait for reload }; diff --git a/grafana-plugin/e2e-tests/utils/schedule.ts b/grafana-plugin/e2e-tests/utils/schedule.ts index e0e64f4424..c08172f199 100644 --- a/grafana-plugin/e2e-tests/utils/schedule.ts +++ b/grafana-plugin/e2e-tests/utils/schedule.ts @@ -4,10 +4,11 @@ import dayjs from 'dayjs'; import { clickButton, selectDropdownValue } from './forms'; import { goToOnCallPage } from './navigation'; -export const createOnCallScheduleWithRotation = async ( +export const createOnCallSchedule = async ( page: Page, scheduleName: string, - userName: string + userName: string, + withRotation = true ): Promise => { // go to the schedules page await goToOnCallPage(page, 'schedules'); @@ -22,7 +23,9 @@ export const createOnCallScheduleWithRotation = async ( // Add a new layer w/ the current user to it await clickButton({ page, buttonText: 'Create Schedule' }); - await createRotation(page, userName); + if (withRotation) { + await createRotation(page, userName); + } }; export const createRotation = async (page: Page, userName: string, isFirstScheduleRotation = true) => { diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts index da86c3b551..98597fcb79 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts @@ -1,4 +1,8 @@ import { Dayjs, ManipulateType } from 'dayjs'; +import { DraggableData } from 'react-draggable'; + +import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; +import { GRAFANA_HEADER_HEIGHT, GRAFANA_LEGACY_SIDEBAR_WIDTH } from 'utils/consts'; import { RepeatEveryPeriod } from './RotationForm.types'; @@ -173,3 +177,40 @@ export const dayJSAddWithDSTFixed = ({ return newDateCandidate.add(diff, 'minutes'); }; + +export function getDraggableModalCoordinatesOnInit( + data: DraggableData, + offsetTop: number +): { + left: number; + right: number; + top: number; + bottom: number; +} { + if (!data) { + return undefined; + } + + const scrollBarReferenceElements = document.querySelectorAll('.scrollbar-view'); + // top navbar display has 2 scrollbar-view elements (navbar & content) + const baseReferenceElRect = ( + scrollBarReferenceElements.length === 1 ? scrollBarReferenceElements[0] : scrollBarReferenceElements[1] + ).getBoundingClientRect(); + + const { right, bottom } = baseReferenceElRect; + + return isTopNavbar() + ? { + // values are adjusted by any padding/margin differences + left: -data.node.offsetLeft + 4, + right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, + top: -offsetTop + GRAFANA_HEADER_HEIGHT + 4, + bottom: bottom - data.node.offsetHeight - offsetTop - 12, + } + : { + left: -data.node.offsetLeft + 4 + GRAFANA_LEGACY_SIDEBAR_WIDTH, + right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, + top: -offsetTop + 4, + bottom: bottom - data.node.offsetHeight - offsetTop - 12, + }; +} diff --git a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx index b068f68e94..cc609ac4a8 100644 --- a/grafana-plugin/src/containers/RotationForm/RotationForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/RotationForm.tsx @@ -40,6 +40,7 @@ import { TimeUnit, timeUnitsToSeconds, TIME_UNITS_ORDER, + getDraggableModalCoordinatesOnInit, } from 'containers/RotationForm/RotationForm.helpers'; import { RepeatEveryPeriod } from 'containers/RotationForm/RotationForm.types'; import { DateTimePicker } from 'containers/RotationForm/parts/DateTimePicker'; @@ -47,6 +48,7 @@ import { DaysSelector } from 'containers/RotationForm/parts/DaysSelector'; import { DeletionModal } from 'containers/RotationForm/parts/DeletionModal'; import { TimeUnitSelector } from 'containers/RotationForm/parts/TimeUnitSelector'; import { UserItem } from 'containers/RotationForm/parts/UserItem'; +import { calculateScheduleFormOffset } from 'containers/Rotations/Rotations.helpers'; import { getShiftName } from 'models/schedule/schedule.helpers'; import { Schedule, Shift } from 'models/schedule/schedule.types'; import { ApiSchemas } from 'network/oncall-api/api.types'; @@ -58,12 +60,11 @@ import { getUTCWeekStart, getWeekStartString, toDateWithTimezoneOffset, + toDateWithTimezoneOffsetAtMidnight, } from 'pages/schedule/Schedule.helpers'; -import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; import { useStore } from 'state/useStore'; -import { getCoords, waitForElement } from 'utils/DOM'; -import { GRAFANA_HEADER_HEIGHT, GRAFANA_LEGACY_SIDEBAR_WIDTH } from 'utils/consts'; -import { useDebouncedCallback } from 'utils/hooks'; +import { GRAFANA_HEADER_HEIGHT } from 'utils/consts'; +import { useDebouncedCallback, useResize } from 'utils/hooks'; import styles from './RotationForm.module.css'; @@ -85,17 +86,11 @@ interface RotationFormProps { const getStartShift = (start: dayjs.Dayjs, timezoneOffset: number, isNewRotation = false) => { if (isNewRotation) { - // all new rotations default to midnight in selected timezone offset - return toDateWithTimezoneOffset(start, timezoneOffset) - .set('date', 1) - .set('year', start.year()) - .set('month', start.month()) - .set('date', start.date()) - .set('hour', 0) - .set('minute', 0) - .set('second', 0); + // default to midnight for new rotations + return toDateWithTimezoneOffsetAtMidnight(start, timezoneOffset); } + // not always midnight return toDateWithTimezoneOffset(start, timezoneOffset); }; @@ -156,12 +151,7 @@ export const RotationForm = observer((props: RotationFormProps) => { const [showDeleteRotationConfirmation, setShowDeleteRotationConfirmation] = useState(false); const debouncedOnResize = useDebouncedCallback(onResize, 250); - useEffect(() => { - window.addEventListener('resize', debouncedOnResize); - return () => { - window.removeEventListener('resize', debouncedOnResize); - }; - }, []); + useResize(debouncedOnResize); useEffect(() => { if (rotationStart.isBefore(shiftStart)) { @@ -178,7 +168,7 @@ export const RotationForm = observer((props: RotationFormProps) => { useEffect(() => { (async () => { if (isOpen) { - setOffsetTop(await calculateOffsetTop()); + setOffsetTop(await calculateScheduleFormOffset(`.${cx('draggable')}`)); } })(); }, [isOpen]); @@ -612,6 +602,7 @@ export const RotationForm = observer((props: RotationFormProps) => { Starts } + data-testid="rotation-start" > { /> } + data-testid="rotation-end" > {endLess ? (
@@ -771,7 +763,9 @@ export const RotationForm = observer((props: RotationFormProps) => {
- Current timezone: {store.timezoneStore.selectedTimezoneLabel} + + Current timezone: {store.timezoneStore.selectedTimezoneLabel} + {shiftId !== 'new' && ( @@ -803,20 +797,9 @@ export const RotationForm = observer((props: RotationFormProps) => { ); async function onResize() { - setDraggablePosition({ x: 0, y: await calculateOffsetTop() }); - } + setOffsetTop(await calculateScheduleFormOffset(`.${cx('draggable')}`)); - async function calculateOffsetTop(): Promise { - const elm = await waitForElement(`#layer${shiftId === 'new' ? layerPriority : shift?.priority_level}`); - const modal = document.querySelector(`.${cx('draggable')}`) as HTMLDivElement; - const coords = getCoords(elm); - - const offsetTop = Math.max( - Math.min(coords.top - modal?.offsetHeight - 10, document.body.offsetHeight - modal?.offsetHeight - 10), - GRAFANA_HEADER_HEIGHT + 10 - ); - - return offsetTop; + setDraggablePosition({ x: 0, y: 0 }); } function onDraggableInit(_e: DraggableEvent, data: DraggableData) { @@ -824,30 +807,7 @@ export const RotationForm = observer((props: RotationFormProps) => { return; } - const scrollBarReferenceElements = document.querySelectorAll('.scrollbar-view'); - // top navbar display has 2 scrollbar-view elements (navbar & content) - const baseReferenceElRect = ( - scrollBarReferenceElements.length === 1 ? scrollBarReferenceElements[0] : scrollBarReferenceElements[1] - ).getBoundingClientRect(); - - const { right, bottom } = baseReferenceElRect; - - setDraggableBounds( - isTopNavbar() - ? { - // values are adjusted by any padding/margin differences - left: -data.node.offsetLeft + 4, - right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, - top: -offsetTop + GRAFANA_HEADER_HEIGHT + 4, - bottom: bottom - data.node.offsetHeight - offsetTop - 12, - } - : { - left: -data.node.offsetLeft + 4 + GRAFANA_LEGACY_SIDEBAR_WIDTH, - right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12, - top: -offsetTop + 4, - bottom: bottom - data.node.offsetHeight - offsetTop - 12, - } - ); + setDraggableBounds(getDraggableModalCoordinatesOnInit(data, offsetTop)); } }); diff --git a/grafana-plugin/src/containers/RotationForm/ScheduleOverrideForm.tsx b/grafana-plugin/src/containers/RotationForm/ScheduleOverrideForm.tsx index fcd6ce74f8..beda68590e 100644 --- a/grafana-plugin/src/containers/RotationForm/ScheduleOverrideForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/ScheduleOverrideForm.tsx @@ -3,22 +3,22 @@ import React, { FC, useCallback, useEffect, useMemo, useState } from 'react'; import { IconButton, VerticalGroup, HorizontalGroup, Field, Button, useTheme2 } from '@grafana/ui'; import cn from 'classnames/bind'; import dayjs from 'dayjs'; -import Draggable from 'react-draggable'; +import Draggable, { DraggableData, DraggableEvent } from 'react-draggable'; import { Modal } from 'components/Modal/Modal'; import { Tag } from 'components/Tag/Tag'; import { Text } from 'components/Text/Text'; import { UserGroups } from 'components/UserGroups/UserGroups'; import { WithConfirm } from 'components/WithConfirm/WithConfirm'; +import { calculateScheduleFormOffset } from 'containers/Rotations/Rotations.helpers'; import { getShiftName } from 'models/schedule/schedule.helpers'; import { Schedule, Shift } from 'models/schedule/schedule.types'; import { ApiSchemas } from 'network/oncall-api/api.types'; -import { getDateTime, getUTCString } from 'pages/schedule/Schedule.helpers'; +import { getDateTime, getUTCString, toDateWithTimezoneOffset } from 'pages/schedule/Schedule.helpers'; import { useStore } from 'state/useStore'; -import { HTML_ID, getCoords, waitForElement } from 'utils/DOM'; -import { GRAFANA_HEADER_HEIGHT } from 'utils/consts'; -import { useDebouncedCallback } from 'utils/hooks'; +import { useDebouncedCallback, useResize } from 'utils/hooks'; +import { getDraggableModalCoordinatesOnInit } from './RotationForm.helpers'; import { DateTimePicker } from './parts/DateTimePicker'; import { UserItem } from './parts/UserItem'; @@ -39,6 +39,9 @@ interface RotationFormProps { const cx = cn.bind(styles); export const ScheduleOverrideForm: FC = (props) => { + const store = useStore(); + const theme = useTheme2(); + const { onHide, onCreate, @@ -46,16 +49,18 @@ export const ScheduleOverrideForm: FC = (props) => { onUpdate, onDelete, shiftId, - shiftStart: propsShiftStart = dayjs().startOf('day').add(1, 'day'), + shiftStart: propsShiftStart = store.timezoneStore.calendarStartDate, shiftEnd: propsShiftEnd, shiftColor: shiftColorProp, } = props; - const store = useStore(); - const theme = useTheme2(); - const [rotationName, setRotationName] = useState(shiftId === 'new' ? 'Override' : 'Update override'); + const [draggablePosition, setDraggablePosition] = useState<{ x: number; y: number }>(undefined); + const [bounds, setDraggableBounds] = useState<{ left: number; right: number; top: number; bottom: number }>( + undefined + ); + const [shiftStart, setShiftStart] = useState(propsShiftStart); const [shiftEnd, setShiftEnd] = useState(propsShiftEnd || propsShiftStart.add(24, 'hours')); @@ -66,6 +71,10 @@ export const ScheduleOverrideForm: FC = (props) => { const [errors, setErrors] = useState<{ [key: string]: string[] }>({}); const shiftColor = shiftColorProp || theme.colors.warning.main; + const debouncedOnResize = useDebouncedCallback(onResize, 250); + + useResize(debouncedOnResize); + const updateShiftStart = useCallback( (value) => { const diff = shiftEnd.diff(shiftStart); @@ -79,15 +88,7 @@ export const ScheduleOverrideForm: FC = (props) => { useEffect(() => { (async () => { if (isOpen) { - const elm = await waitForElement(`#${HTML_ID.SCHEDULE_OVERRIDES_AND_SWAPS}`); - const modal = document.querySelector(`.${cx('draggable')}`) as HTMLDivElement; - const coords = getCoords(elm); - const offsetTop = Math.min( - Math.max(coords.top - modal?.offsetHeight - 10, GRAFANA_HEADER_HEIGHT + 10), - document.body.offsetHeight - modal?.offsetHeight - 10 - ); - - setOffsetTop(offsetTop); + setOffsetTop(await calculateScheduleFormOffset(`.${cx('draggable')}`)); } })(); }, [isOpen]); @@ -102,6 +103,11 @@ export const ScheduleOverrideForm: FC = (props) => { } }, [shiftId]); + useEffect(() => { + setShiftStart(toDateWithTimezoneOffset(shiftStart, store.timezoneStore.selectedTimezoneOffset)); + setShiftEnd(toDateWithTimezoneOffset(shiftEnd, store.timezoneStore.selectedTimezoneOffset)); + }, [store.timezoneStore.selectedTimezoneOffset]); + const params = useMemo( () => ({ rotation_start: getUTCString(shiftStart), @@ -200,7 +206,15 @@ export const ScheduleOverrideForm: FC = (props) => { width="430px" onDismiss={onHide} contentElement={(props, children) => ( - + setDraggablePosition({ x: data.x, y: data.y })} + >
{children}
 )} @@ -215,7 +229,7 @@ export const ScheduleOverrideForm: FC = (props) => { {shiftId !== 'new' && ( - + )} @@ -228,49 +242,70 @@ export const ScheduleOverrideForm: FC = (props) => { /> -
- - - - Override period start - - } - > - - - - Override period end - - } - > - - - - ( - - )} - showError={Boolean(errors.rolling_users)} - /> - + +
+
+ + + + Override period start + + } + > + + + + + Override period end + + } + > + + + + + ( + + )} + showError={Boolean(errors.rolling_users)} + /> + +
- Current timezone: {store.timezoneStore.selectedTimezoneLabel} + + Current timezone: {store.timezoneStore.selectedTimezoneLabel} +