Handle slack payload metadata limit in paging command (#2007)
Fixes https://github.com/grafana/oncall-private/issues/1838
This commit is contained in:
parent
a536af95f4
commit
79432bc3bc
4 changed files with 140 additions and 10 deletions
|
|
@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
### Fixed
|
||||
|
||||
- Handle slack metadata limit when creating paging command payload ([#2007](https://github.com/grafana/oncall/pull/2007))
|
||||
- Fix issue with sometimes cached final schedule not being refreshed after an update ([#2004](https://github.com/grafana/oncall/pull/2004))
|
||||
|
||||
## v1.2.28 (2023-05-24)
|
||||
|
|
|
|||
|
|
@ -9,3 +9,5 @@ SLACK_WRONG_TEAM_NAMES = [SLACK_INVALID_AUTH_RESPONSE, PLACEHOLDER]
|
|||
SLACK_RATE_LIMIT_TIMEOUT = timezone.timedelta(minutes=5)
|
||||
SLACK_RATE_LIMIT_DELAY = 10
|
||||
CACHE_UPDATE_INCIDENT_SLACK_MESSAGE_LIFETIME = 60 * 10
|
||||
|
||||
PRIVATE_METADATA_MAX_LENGTH = 3000
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from apps.alerts.paging import (
|
|||
check_user_availability,
|
||||
direct_paging,
|
||||
)
|
||||
from apps.slack.constants import PRIVATE_METADATA_MAX_LENGTH
|
||||
from apps.slack.models import SlackChannel
|
||||
from apps.slack.scenarios import scenario_step
|
||||
from apps.slack.slack_client.exceptions import SlackAPIException
|
||||
|
|
@ -47,7 +48,10 @@ USERS_DATA_KEY = "users"
|
|||
def add_or_update_item(payload, key, item_pk, policy):
|
||||
metadata = json.loads(payload["view"]["private_metadata"])
|
||||
metadata[key][item_pk] = policy
|
||||
payload["view"]["private_metadata"] = json.dumps(metadata)
|
||||
updated_metadata = json.dumps(metadata)
|
||||
if len(updated_metadata) > PRIVATE_METADATA_MAX_LENGTH:
|
||||
raise ValueError("Cannot add entry, maximum exceeded")
|
||||
payload["view"]["private_metadata"] = updated_metadata
|
||||
return payload
|
||||
|
||||
|
||||
|
|
@ -213,8 +217,13 @@ class OnPagingUserChange(scenario_step.ScenarioStep):
|
|||
)
|
||||
else:
|
||||
# user is available to be paged
|
||||
updated_payload = add_or_update_item(payload, USERS_DATA_KEY, selected_user.pk, DEFAULT_POLICY)
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload)
|
||||
error_msg = None
|
||||
try:
|
||||
updated_payload = add_or_update_item(payload, USERS_DATA_KEY, selected_user.pk, DEFAULT_POLICY)
|
||||
except ValueError:
|
||||
updated_payload = payload
|
||||
error_msg = "Cannot add user, maximum responders exceeded"
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload, error_msg=error_msg)
|
||||
self._slack_client.api_call(
|
||||
"views.update",
|
||||
trigger_id=payload["trigger_id"],
|
||||
|
|
@ -233,12 +242,17 @@ class OnPagingItemActionChange(scenario_step.ScenarioStep):
|
|||
def process_scenario(self, slack_user_identity, slack_team_identity, payload, policy=None):
|
||||
policy, key, user_pk = self._parse_action(payload)
|
||||
|
||||
error_msg = None
|
||||
if policy == REMOVE_ACTION:
|
||||
updated_payload = remove_item(payload, key, user_pk)
|
||||
else:
|
||||
updated_payload = add_or_update_item(payload, key, user_pk, policy)
|
||||
try:
|
||||
updated_payload = add_or_update_item(payload, key, user_pk, policy)
|
||||
except ValueError:
|
||||
updated_payload = payload
|
||||
error_msg = "Cannot update policy, maximum responders exceeded"
|
||||
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload)
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload, error_msg=error_msg)
|
||||
self._slack_client.api_call(
|
||||
"views.update",
|
||||
trigger_id=payload["trigger_id"],
|
||||
|
|
@ -269,8 +283,15 @@ class OnPagingConfirmUserChange(scenario_step.ScenarioStep):
|
|||
}
|
||||
# add selected user
|
||||
selected_user = _get_selected_user_from_payload(previous_view_payload, private_metadata["input_id_prefix"])
|
||||
updated_payload = add_or_update_item(previous_view_payload, USERS_DATA_KEY, selected_user.pk, DEFAULT_POLICY)
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload)
|
||||
error_msg = None
|
||||
try:
|
||||
updated_payload = add_or_update_item(
|
||||
previous_view_payload, USERS_DATA_KEY, selected_user.pk, DEFAULT_POLICY
|
||||
)
|
||||
except ValueError:
|
||||
updated_payload = payload
|
||||
error_msg = "Cannot add user, maximum responders exceeded"
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload, error_msg=error_msg)
|
||||
self._slack_client.api_call(
|
||||
"views.update",
|
||||
trigger_id=payload["trigger_id"],
|
||||
|
|
@ -291,8 +312,13 @@ class OnPagingScheduleChange(scenario_step.ScenarioStep):
|
|||
if selected_schedule is None:
|
||||
return
|
||||
|
||||
updated_payload = add_or_update_item(payload, SCHEDULES_DATA_KEY, selected_schedule.pk, DEFAULT_POLICY)
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload)
|
||||
error_msg = None
|
||||
try:
|
||||
updated_payload = add_or_update_item(payload, SCHEDULES_DATA_KEY, selected_schedule.pk, DEFAULT_POLICY)
|
||||
except ValueError:
|
||||
updated_payload = payload
|
||||
error_msg = "Cannot add schedule, maximum responders exceeded"
|
||||
view = render_dialog(slack_user_identity, slack_team_identity, updated_payload, error_msg=error_msg)
|
||||
self._slack_client.api_call(
|
||||
"views.update",
|
||||
trigger_id=payload["trigger_id"],
|
||||
|
|
@ -306,7 +332,7 @@ class OnPagingScheduleChange(scenario_step.ScenarioStep):
|
|||
DIVIDER_BLOCK = {"type": "divider"}
|
||||
|
||||
|
||||
def render_dialog(slack_user_identity, slack_team_identity, payload, initial=False):
|
||||
def render_dialog(slack_user_identity, slack_team_identity, payload, initial=False, error_msg=None):
|
||||
private_metadata = json.loads(payload["view"]["private_metadata"])
|
||||
submit_routing_uid = private_metadata.get("submit_routing_uid")
|
||||
if initial:
|
||||
|
|
@ -345,6 +371,18 @@ def render_dialog(slack_user_identity, slack_team_identity, payload, initial=Fal
|
|||
# blocks
|
||||
blocks = [organization_select, team_select, escalation_select, users_select, schedules_select]
|
||||
|
||||
if error_msg:
|
||||
blocks += [
|
||||
{
|
||||
"type": "section",
|
||||
"block_id": "error_message",
|
||||
"text": {
|
||||
"type": "mrkdwn",
|
||||
"text": f":warning: {error_msg}",
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
# selected items
|
||||
selected_users = get_current_items(payload, USERS_DATA_KEY, selected_organization.users)
|
||||
selected_schedules = get_current_items(payload, SCHEDULES_DATA_KEY, selected_organization.oncall_schedules)
|
||||
|
|
|
|||
|
|
@ -138,6 +138,63 @@ def test_add_user_no_warning(
|
|||
assert metadata[USERS_DATA_KEY] == {str(user.pk): DEFAULT_POLICY}
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_add_user_maximum_exceeded(
|
||||
make_organization_and_user_with_slack_identities, make_schedule, make_on_call_shift, make_user_notification_policy
|
||||
):
|
||||
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
|
||||
# set up schedule: user is on call
|
||||
schedule = make_schedule(
|
||||
organization,
|
||||
schedule_class=OnCallScheduleWeb,
|
||||
team=None,
|
||||
)
|
||||
now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
|
||||
start_date = now - timezone.timedelta(days=7)
|
||||
data = {
|
||||
"start": start_date,
|
||||
"rotation_start": start_date,
|
||||
"duration": timezone.timedelta(hours=23, minutes=59, seconds=59),
|
||||
"priority_level": 1,
|
||||
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
|
||||
"schedule": schedule,
|
||||
}
|
||||
on_call_shift = make_on_call_shift(
|
||||
organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
|
||||
)
|
||||
on_call_shift.add_rolling_users([[user]])
|
||||
schedule.refresh_ical_file()
|
||||
# setup notification policy
|
||||
make_user_notification_policy(
|
||||
user=user,
|
||||
step=UserNotificationPolicy.Step.NOTIFY,
|
||||
notify_by=UserNotificationPolicy.NotificationChannel.SMS,
|
||||
)
|
||||
|
||||
payload = make_slack_payload(organization=organization, user=user)
|
||||
|
||||
step = OnPagingUserChange(slack_team_identity)
|
||||
with patch("apps.slack.scenarios.paging.PRIVATE_METADATA_MAX_LENGTH", 100):
|
||||
with patch.object(step._slack_client, "api_call") as mock_slack_api_call:
|
||||
step.process_scenario(slack_user_identity, slack_team_identity, payload)
|
||||
|
||||
assert mock_slack_api_call.call_args.args == ("views.update",)
|
||||
view_data = mock_slack_api_call.call_args.kwargs["view"]
|
||||
metadata = json.loads(view_data["private_metadata"])
|
||||
# metadata unchanged, ignoring the prefix
|
||||
original_metadata = json.loads(payload["view"]["private_metadata"])
|
||||
metadata.pop("input_id_prefix")
|
||||
original_metadata.pop("input_id_prefix")
|
||||
assert metadata == original_metadata
|
||||
# error message is displayed
|
||||
error_block = {
|
||||
"type": "section",
|
||||
"block_id": "error_message",
|
||||
"text": {"type": "mrkdwn", "text": ":warning: Cannot add user, maximum responders exceeded"},
|
||||
}
|
||||
assert error_block in view_data["blocks"]
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_add_user_raise_warning(make_organization_and_user_with_slack_identities):
|
||||
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
|
||||
|
|
@ -247,6 +304,38 @@ def test_add_schedule(make_organization_and_user_with_slack_identities, make_sch
|
|||
assert metadata[USERS_DATA_KEY] == {str(user.pk): IMPORTANT_POLICY}
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_add_schedule_responders_exceeded(make_organization_and_user_with_slack_identities, make_schedule):
|
||||
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
|
||||
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, team=None)
|
||||
payload = make_slack_payload(
|
||||
organization=organization,
|
||||
schedule=schedule,
|
||||
current_users={str(user.pk): IMPORTANT_POLICY},
|
||||
)
|
||||
|
||||
step = OnPagingScheduleChange(slack_team_identity)
|
||||
with patch("apps.slack.scenarios.paging.PRIVATE_METADATA_MAX_LENGTH", 100):
|
||||
with patch.object(step._slack_client, "api_call") as mock_slack_api_call:
|
||||
step.process_scenario(slack_user_identity, slack_team_identity, payload)
|
||||
|
||||
assert mock_slack_api_call.call_args.args == ("views.update",)
|
||||
view_data = mock_slack_api_call.call_args.kwargs["view"]
|
||||
metadata = json.loads(view_data["private_metadata"])
|
||||
# metadata unchanged, ignoring the prefix
|
||||
original_metadata = json.loads(payload["view"]["private_metadata"])
|
||||
metadata.pop("input_id_prefix")
|
||||
original_metadata.pop("input_id_prefix")
|
||||
assert metadata == original_metadata
|
||||
# error message is displayed
|
||||
error_block = {
|
||||
"type": "section",
|
||||
"block_id": "error_message",
|
||||
"text": {"type": "mrkdwn", "text": ":warning: Cannot add schedule, maximum responders exceeded"},
|
||||
}
|
||||
assert error_block in view_data["blocks"]
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_change_schedule_policy(make_organization_and_user_with_slack_identities, make_schedule):
|
||||
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue