diff --git a/CHANGELOG.md b/CHANGELOG.md index ec85c67e..ce5b9778 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 +### Added + +- Added the ability to change the team for escalation chains + ### Fixed - Addressed bug with iOS mobile push notifications always being set to critical by @imtoori and @joeyorlando ([#1646](https://github.com/grafana/oncall/pull/1646)) diff --git a/engine/apps/alerts/models/escalation_chain.py b/engine/apps/alerts/models/escalation_chain.py index 9fe65424..2dcd7254 100644 --- a/engine/apps/alerts/models/escalation_chain.py +++ b/engine/apps/alerts/models/escalation_chain.py @@ -46,11 +46,11 @@ class EscalationChain(models.Model): def __str__(self): return f"{self.pk}: {self.name}" - def make_copy(self, copy_name: str): + def make_copy(self, copy_name: str, team): with transaction.atomic(): copied_chain = EscalationChain.objects.create( organization=self.organization, - team=self.team, + team=team, name=copy_name, ) for escalation_policy in self.escalation_policies.all(): diff --git a/engine/apps/alerts/tests/test_escalation_chain.py b/engine/apps/alerts/tests/test_escalation_chain.py index c70371cd..8f723c7a 100644 --- a/engine/apps/alerts/tests/test_escalation_chain.py +++ b/engine/apps/alerts/tests/test_escalation_chain.py @@ -42,7 +42,7 @@ def test_copy_escalation_chain( all_fields = EscalationPolicy._meta.fields # Note that m-t-m fields are in this list fields_to_not_compare = ["id", "public_primary_key", "escalation_chain", "last_notified_user"] fields_to_compare = list(map(lambda f: f.name, filter(lambda f: f.name not in fields_to_not_compare, all_fields))) - copied_chain = escalation_chain.make_copy(f"copy_{escalation_chain.name}") + copied_chain = escalation_chain.make_copy(f"copy_{escalation_chain.name}", None) for policy_from_original, policy_from_copy in zip( escalation_chain.escalation_policies.all(), copied_chain.escalation_policies.all() ): diff --git a/engine/apps/api/tests/test_escalation_chain.py b/engine/apps/api/tests/test_escalation_chain.py index a9dcec1d..9d06af88 100644 --- a/engine/apps/api/tests/test_escalation_chain.py +++ b/engine/apps/api/tests/test_escalation_chain.py @@ -74,3 +74,57 @@ def test_list_escalation_chains_filters(escalation_chain_internal_api_setup, mak "display_name": escalation_chain.name, } ] + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "team_name,new_team_name", + [ + (None, None), + (None, "team_1"), + ("team_1", None), + ("team_1", "team_1"), + ("team_1", "team_2"), + ], +) +def test_escalation_chain_copy( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + make_escalation_chain, + make_team, + team_name, + new_team_name, +): + organization, user, token = make_organization_and_user_with_plugin_token() + + team = make_team(organization, name=team_name) if team_name else None + new_team = make_team(organization, name=new_team_name) if new_team_name else None + + escalation_chain = make_escalation_chain(organization, team=team) + data = { + "name": "escalation_chain_updated", + "team": new_team.public_primary_key if new_team else "null", + } + + client = APIClient() + url = reverse("api-internal:escalation_chain-copy", kwargs={"pk": escalation_chain.public_primary_key}) + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert response.json()["team"] == (new_team.public_primary_key if new_team else None) + + +@pytest.mark.django_db +def test_escalation_chain_copy_empty_name( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + make_escalation_chain, +): + organization, user, token = make_organization_and_user_with_plugin_token() + escalation_chain = make_escalation_chain(organization) + + client = APIClient() + url = reverse("api-internal:escalation_chain-copy", kwargs={"pk": escalation_chain.public_primary_key}) + + response = client.post(url, {"name": "", "team": "null"}, format="json", **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/api/views/escalation_chain.py b/engine/apps/api/views/escalation_chain.py index fab3c102..6be5a7bf 100644 --- a/engine/apps/api/views/escalation_chain.py +++ b/engine/apps/api/views/escalation_chain.py @@ -1,7 +1,7 @@ from django.db.models import Count, Q from django_filters import rest_framework as filters from emoji import emojize -from rest_framework import viewsets +from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.filters import SearchFilter from rest_framework.permissions import IsAuthenticated @@ -15,6 +15,7 @@ from apps.api.serializers.escalation_chain import ( FilterEscalationChainSerializer, ) from apps.auth_token.auth import PluginAuthentication +from apps.user_management.models import Team from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamModelFieldFilterMixin, ModelFieldFilterMixin, TeamModelMultipleChoiceFilter from common.api_helpers.mixins import ( @@ -120,14 +121,22 @@ class EscalationChainViewSet( @action(methods=["post"], detail=True) def copy(self, request, pk): name = request.data.get("name") - if name is None: + team_id = request.data.get("team") + if team_id == "null": + team_id = None + + if not name: raise BadRequest(detail={"name": ["This field may not be null."]}) else: if EscalationChain.objects.filter(organization=request.auth.organization, name=name).exists(): raise BadRequest(detail={"name": ["Escalation chain with this name already exists."]}) obj = self.get_object() - copy = obj.make_copy(name) + try: + team = request.user.available_teams.get(public_primary_key=team_id) if team_id else None + except Team.DoesNotExist: + return Response(data={"error_code": "wrong_team"}, status=status.HTTP_403_FORBIDDEN) + copy = obj.make_copy(name, team) serializer = self.get_serializer(copy) write_resource_insight_log( instance=copy, diff --git a/grafana-plugin/src/containers/AlertRules/AlertRules.tsx b/grafana-plugin/src/containers/AlertRules/AlertRules.tsx index 5d373cf0..9788204c 100644 --- a/grafana-plugin/src/containers/AlertRules/AlertRules.tsx +++ b/grafana-plugin/src/containers/AlertRules/AlertRules.tsx @@ -28,7 +28,7 @@ import WithConfirm from 'components/WithConfirm/WithConfirm'; import { parseEmojis } from 'containers/AlertRules/AlertRules.helpers'; import { ChatOpsConnectors } from 'containers/AlertRules/parts'; import ChannelFilterForm from 'containers/ChannelFilterForm/ChannelFilterForm'; -import EscalationChainForm from 'containers/EscalationChainForm/EscalationChainForm'; +import EscalationChainForm, { EscalationChainFormMode } from 'containers/EscalationChainForm/EscalationChainForm'; import EscalationChainSteps from 'containers/EscalationChainSteps/EscalationChainSteps'; import GSelect from 'containers/GSelect/GSelect'; import { IntegrationSettingsTab } from 'containers/IntegrationSettings/IntegrationSettings.types'; @@ -348,6 +348,7 @@ class AlertRules extends React.Component { )} {channelFilterIdToCopyEscalationChain && ( { this.setState({ diff --git a/grafana-plugin/src/containers/EscalationChainForm/EscalationChainForm.tsx b/grafana-plugin/src/containers/EscalationChainForm/EscalationChainForm.tsx index fea35b7e..26fa138d 100644 --- a/grafana-plugin/src/containers/EscalationChainForm/EscalationChainForm.tsx +++ b/grafana-plugin/src/containers/EscalationChainForm/EscalationChainForm.tsx @@ -3,15 +3,22 @@ import React, { ChangeEvent, FC, useCallback, useState } from 'react'; import { Button, Field, HorizontalGroup, Input, Modal } from '@grafana/ui'; import cn from 'classnames/bind'; -import GrafanaTeamSelect from 'containers/GrafanaTeamSelect/GrafanaTeamSelect'; +import GSelect from 'containers/GSelect/GSelect'; import { EscalationChain } from 'models/escalation_chain/escalation_chain.types'; import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; import { useStore } from 'state/useStore'; import styles from 'containers/EscalationChainForm/EscalationChainForm.module.css'; +export enum EscalationChainFormMode { + Create = 'Create', + Copy = 'Copy', + Update = 'Update', +} + interface EscalationChainFormProps { escalationChainId?: EscalationChain['id']; + mode: EscalationChainFormMode; onHide: () => void; onUpdate: (id: EscalationChain['id']) => void; } @@ -19,7 +26,7 @@ interface EscalationChainFormProps { const cx = cn.bind(styles); const EscalationChainForm: FC = (props) => { - const { escalationChainId, onHide, onUpdate } = props; + const { escalationChainId, onHide, onUpdate, mode } = props; const store = useStore(); const { escalationChainStore, userStore } = store; @@ -28,15 +35,21 @@ const EscalationChainForm: FC = (props) => { const escalationChain = escalationChainId ? escalationChainStore.items[escalationChainId] : undefined; - const [name, setName] = useState(escalationChain?.name); - const [selectedTeam, setSelectedTeam] = useState(user.current_team); + const [name, setName] = useState( + mode === EscalationChainFormMode.Copy ? `${escalationChain?.name} copy` : escalationChain?.name + ); + const [selectedTeam, setSelectedTeam] = useState(escalationChain?.team || user.current_team); const [errors, setErrors] = useState<{ [key: string]: string }>({}); const onCreateClickCallback = useCallback(() => { - (escalationChainId - ? escalationChainStore.clone(escalationChainId, { name, team: selectedTeam }) - : escalationChainStore.create({ name, team: selectedTeam }) - ) + const promise = + mode === EscalationChainFormMode.Create + ? escalationChainStore.create({ name, team: selectedTeam }) + : mode === EscalationChainFormMode.Copy + ? escalationChainStore.clone(escalationChainId, { name, team: selectedTeam }) + : escalationChainStore.update(escalationChainId, { name, team: selectedTeam }); + + promise .then((escalationChain: EscalationChain) => { onUpdate(escalationChain.id); @@ -47,21 +60,27 @@ const EscalationChainForm: FC = (props) => { name: data.response.data.name || data.response.data.detail || data.response.data.non_field_errors, }); }); - }, [name, selectedTeam]); + }, [name, selectedTeam, mode]); const handleNameChange = useCallback((event: ChangeEvent) => { setName(event.target.value); }, []); return ( - +
- + = (props) => { Cancel
diff --git a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx index 932838f4..2163c779 100644 --- a/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx +++ b/grafana-plugin/src/containers/RemoteFilters/RemoteFilters.tsx @@ -184,7 +184,7 @@ class RemoteFilters extends Component { if (option.data.default) { let defaultValue = option.data.default; if (option.data.type === 'options') { - defaultValue = [defaultValue]; + defaultValue = [option.data.default.value]; } if (option.data.type === 'boolean') { defaultValue = defaultValue === 'false' ? false : Boolean(defaultValue); diff --git a/grafana-plugin/src/pages/escalation-chains/EscalationChains.tsx b/grafana-plugin/src/pages/escalation-chains/EscalationChains.tsx index c7b8d67a..5ebeb3b6 100644 --- a/grafana-plugin/src/pages/escalation-chains/EscalationChains.tsx +++ b/grafana-plugin/src/pages/escalation-chains/EscalationChains.tsx @@ -19,7 +19,7 @@ import Tutorial from 'components/Tutorial/Tutorial'; import { TutorialStep } from 'components/Tutorial/Tutorial.types'; import WithConfirm from 'components/WithConfirm/WithConfirm'; import EscalationChainCard from 'containers/EscalationChainCard/EscalationChainCard'; -import EscalationChainForm from 'containers/EscalationChainForm/EscalationChainForm'; +import EscalationChainForm, { EscalationChainFormMode } from 'containers/EscalationChainForm/EscalationChainForm'; import EscalationChainSteps from 'containers/EscalationChainSteps/EscalationChainSteps'; import RemoteFilters from 'containers/RemoteFilters/RemoteFilters'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; @@ -37,11 +37,10 @@ const cx = cn.bind(styles); interface EscalationChainsPageProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} interface EscalationChainsPageState extends PageBaseState { - showCreateEscalationChainModal: boolean; - escalationChainIdToCopy: EscalationChain['id']; + modeToShowEscalationChainForm?: EscalationChainFormMode; selectedEscalationChain: EscalationChain['id']; escalationChainsFilters?: FiltersValues; - extraEscalationChains?: EscalationChain[]; // to render Escalation chain that is not present in searchResult dur to filters + extraEscalationChains?: EscalationChain[]; // to render Escalation chains that are not present in searchResult due to filters } export interface Filters { @@ -51,8 +50,6 @@ export interface Filters { @observer class EscalationChainsPage extends React.Component { state: EscalationChainsPageState = { - showCreateEscalationChainModal: false, - escalationChainIdToCopy: undefined, selectedEscalationChain: undefined, errorData: initErrorDataState(), }; @@ -127,7 +124,7 @@ class EscalationChainsPage extends React.Component