diff --git a/CHANGELOG.md b/CHANGELOG.md index 29d012db..eb8c14d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add backend for multi-stack support for mobile-app @Ferril ([#3500](https://github.com/grafana/oncall/pull/3500)) +### Fixed + +- Fix PUT /api/v1/escalation_policies/id issue when updating `from_time` and `to_time` by @joeyorlando ([#3581](https://github.com/grafana/oncall/pull/3581)) + ## v1.3.78 (2023-12-12) ### Changed diff --git a/engine/apps/public_api/serializers/escalation_policies.py b/engine/apps/public_api/serializers/escalation_policies.py index 35c2b616..a100d6fb 100644 --- a/engine/apps/public_api/serializers/escalation_policies.py +++ b/engine/apps/public_api/serializers/escalation_policies.py @@ -10,7 +10,6 @@ from apps.slack.models import SlackUserGroup from apps.user_management.models import User from apps.webhooks.models import Webhook from common.api_helpers.custom_fields import ( - CustomTimeField, OrganizationFilteredPrimaryKeyRelatedField, UsersFilteredByOrganizationField, ) @@ -78,8 +77,14 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer): source="custom_webhook", ) important = serializers.BooleanField(required=False) - notify_if_time_from = CustomTimeField(required=False, source="from_time") - notify_if_time_to = CustomTimeField(required=False, source="to_time") + + TIME_FORMAT = "%H:%M:%SZ" + notify_if_time_from = serializers.TimeField( + required=False, source="from_time", format=TIME_FORMAT, input_formats=[TIME_FORMAT] + ) + notify_if_time_to = serializers.TimeField( + required=False, source="to_time", format=TIME_FORMAT, input_formats=[TIME_FORMAT] + ) class Meta: model = EscalationPolicy diff --git a/engine/apps/public_api/tests/test_escalation_policies.py b/engine/apps/public_api/tests/test_escalation_policies.py index 1a9a19c2..a04a2440 100644 --- a/engine/apps/public_api/tests/test_escalation_policies.py +++ b/engine/apps/public_api/tests/test_escalation_policies.py @@ -418,3 +418,41 @@ def test_update_escalation_policy_using_button_to_webhook( assert response.data == serializer.data # step is migrated assert escalation_policy.step == EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "value,expected_status", + [ + (5, status.HTTP_400_BAD_REQUEST), + ("5", status.HTTP_400_BAD_REQUEST), + ("5:00", status.HTTP_400_BAD_REQUEST), + ("05:00:00", status.HTTP_400_BAD_REQUEST), + ("05:00:00Z", status.HTTP_200_OK), + ], +) +def test_update_escalation_policy_from_and_to_time( + make_organization_and_user_with_token, + make_escalation_chain, + make_escalation_policy, + value, + expected_status, +): + organization, _, token = make_organization_and_user_with_token() + escalation_chain = make_escalation_chain(organization) + escalation_policy = make_escalation_policy(escalation_chain, EscalationPolicy.STEP_NOTIFY_IF_TIME) + + client = APIClient() + url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy.public_primary_key}) + + for field in ["notify_if_time_from", "notify_if_time_to"]: + response = client.put(url, data={field: value}, format="json", HTTP_AUTHORIZATION=token) + + assert response.status_code == expected_status + + if expected_status == status.HTTP_200_OK: + escalation_policy = EscalationPolicy.objects.get(public_primary_key=response.data["id"]) + serializer = EscalationPolicySerializer(escalation_policy) + assert response.data == serializer.data + else: + assert response.json()[field][0] == "Time has wrong format. Use one of these formats instead: hh:mm:ssZ." diff --git a/engine/common/api_helpers/custom_fields.py b/engine/common/api_helpers/custom_fields.py index 32f5736b..dcfc1bb8 100644 --- a/engine/common/api_helpers/custom_fields.py +++ b/engine/common/api_helpers/custom_fields.py @@ -1,5 +1,3 @@ -import time - from django.core.exceptions import ObjectDoesNotExist from rest_framework import fields, serializers from rest_framework.exceptions import ValidationError @@ -102,25 +100,6 @@ class UsersFilteredByOrganizationField(serializers.Field): return queryset.filter(organization=request.user.organization, public_primary_key__in=data).distinct() -class CustomTimeField(fields.TimeField): - def to_representation(self, value): - result = super().to_representation(value) - if result[-1] != "Z": - result += "Z" - return result - - def to_internal_value(self, data): - TIME_FORMAT_LEN = len("00:00:00Z") - if len(data) == TIME_FORMAT_LEN: - try: - time.strptime(data, "%H:%M:%SZ") - except ValueError: - raise BadRequest(detail="Invalid time format, should be '00:00:00Z'") - else: - raise BadRequest(detail="Invalid time format, should be '00:00:00Z'") - return data - - class RouteIdField(fields.CharField): def to_internal_value(self, data): try: diff --git a/grafana-plugin/e2e-tests/escalationChains/escalationPolicy.test.ts b/grafana-plugin/e2e-tests/escalationChains/escalationPolicy.test.ts index 70b10b3e..ecf44db0 100644 --- a/grafana-plugin/e2e-tests/escalationChains/escalationPolicy.test.ts +++ b/grafana-plugin/e2e-tests/escalationChains/escalationPolicy.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from '../fixtures'; +import { Locator, expect, test } from '../fixtures'; import { createEscalationChain, EscalationStep, selectEscalationStepValue } from '../utils/escalationChain'; import { generateRandomValue } from '../utils/forms'; @@ -15,3 +15,45 @@ test('escalation policy does not go back to "Default" after adding users to noti await page.reload(); await expect(page.getByText('Important')).toBeVisible(); }); + +// TODO: unskip when https://github.com/grafana/oncall/issues/3585 is patched +test.skip('from_time and to_time for "Continue escalation if current UTC time is in range" escalation step type can be properly updated', async ({ + adminRolePage, +}) => { + const FROM_TIME = '10:31'; + const TO_TIME = '10:32'; + + const { page } = adminRolePage; + const escalationChainName = generateRandomValue(); + + // create escalation step w/ Continue escalation if current UTC time is in policy step + await createEscalationChain(page, escalationChainName, EscalationStep.ContinueEscalationIfCurrentUTCTimeIsIn); + + const _getFromTimeInput = () => page.locator('[data-testid="time-range-from"] >> input'); + const _getToTimeInput = () => page.locator('[data-testid="time-range-to"] >> input'); + + const clickAndInputValue = async (locator: Locator, value: string) => { + // the first click opens up dropdown which contains the time selector scrollable lists + await locator.click(); + + // the second click focuses on the input where we can actually type the time instead, much easier + const actualInput = page.locator('input[class="rc-time-picker-panel-input"]'); + await actualInput.click(); + await actualInput.selectText(); + await actualInput.fill(value); + + // click anywhere to close the dropdown + await page.click('body'); + }; + + // update from and to time values + await clickAndInputValue(_getFromTimeInput(), FROM_TIME); + await clickAndInputValue(_getToTimeInput(), TO_TIME); + + // reload and check that these values have been persisted + await page.reload(); + await page.waitForLoadState('networkidle'); + + expect(await _getFromTimeInput().textContent()).toBe(FROM_TIME); + expect(await _getToTimeInput().textContent()).toBe(FROM_TIME); +}); diff --git a/grafana-plugin/e2e-tests/utils/escalationChain.ts b/grafana-plugin/e2e-tests/utils/escalationChain.ts index 74115d43..c7e5c772 100644 --- a/grafana-plugin/e2e-tests/utils/escalationChain.ts +++ b/grafana-plugin/e2e-tests/utils/escalationChain.ts @@ -6,9 +6,10 @@ import { goToOnCallPage } from './navigation'; export enum EscalationStep { NotifyUsers = 'Notify users', NotifyUsersFromOnCallSchedule = 'Notify users from on-call schedule', + ContinueEscalationIfCurrentUTCTimeIsIn = 'Continue escalation if current UTC time is in range', } -const escalationStepValuePlaceholder: Record = { +const escalationStepValuePlaceholder: Partial> = { [EscalationStep.NotifyUsers]: 'Select User', [EscalationStep.NotifyUsersFromOnCallSchedule]: 'Select Schedule', }; diff --git a/grafana-plugin/src/components/TimeRange/TimeRange.tsx b/grafana-plugin/src/components/TimeRange/TimeRange.tsx index b48e7435..28736b83 100644 --- a/grafana-plugin/src/components/TimeRange/TimeRange.tsx +++ b/grafana-plugin/src/components/TimeRange/TimeRange.tsx @@ -92,11 +92,15 @@ const TimeRange = (props: TimeRangeProps) => { return (
- {/* @ts-ignore actually TimeOfDayPicker uses Moment objects */} - +
+ {/* @ts-ignore actually TimeOfDayPicker uses Moment objects */} + +
to - {/* @ts-ignore actually TimeOfDayPicker uses Moment objects */} - +
+ {/* @ts-ignore actually TimeOfDayPicker uses Moment objects */} + +
{showNextDayTip && 'next day'}