Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direct paging improvements #2537

Merged
merged 29 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
af013db
fix direct paging dialog not changing values
vstpme Jul 12, 2023
25253b7
Merge branch 'dev' into vadimkerr/deprecate-oncall-slash-commands
vstpme Jul 13, 2023
135b1b4
deprecate /oncall
vstpme Jul 14, 2023
8e90539
update Slack direct paging
vstpme Jul 14, 2023
2c8a065
remove escalation chain stuff
vstpme Jul 14, 2023
563cf32
hide org select if only one org is available
vstpme Jul 14, 2023
7ab6dd6
Merge branch 'dev' into vadimkerr/deprecate-oncall-slash-commands
vstpme Jul 14, 2023
9ab0e6a
no team preselected
vstpme Jul 14, 2023
a33cdf5
unify wording
vstpme Jul 14, 2023
b5a2b36
web UI wording
vstpme Jul 14, 2023
01650dd
web UI wording + allow null message
vstpme Jul 14, 2023
295fbbb
web UI wording
vstpme Jul 14, 2023
2c482eb
web UI wording
vstpme Jul 14, 2023
201b761
web UI wording
vstpme Jul 14, 2023
0af6ce7
web UI wording
vstpme Jul 14, 2023
303a511
more unification
vstpme Jul 14, 2023
baa0db5
show search
vstpme Jul 14, 2023
10b769b
return link to alert group
vstpme Jul 14, 2023
708f466
add some margin between channels
vstpme Jul 14, 2023
9a75f17
fix chatOpsAvailableChannels
vstpme Jul 14, 2023
517373c
optional
vstpme Jul 14, 2023
dbb2378
tests
vstpme Jul 14, 2023
c52c340
tests
vstpme Jul 14, 2023
1014502
bring back team name to integration
vstpme Jul 14, 2023
be132c2
tests
vstpme Jul 14, 2023
daa0722
Merge branch 'dev' into vadimkerr/deprecate-oncall-slash-commands
vstpme Jul 17, 2023
bd06987
changelog
vstpme Jul 17, 2023
9e3aafa
Merge branch 'dev' into vadimkerr/deprecate-oncall-slash-commands
vstpme Jul 17, 2023
c575605
changelog
vstpme Jul 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Remove deprecated `heartbeat.HeartBeat` model/table by @joeyorlando ([#2534](https://github.com/grafana/oncall/pull/2534))
- Deprecate `/oncall` Slack command, update direct paging functionality by @vadimkerr ([#2537](https://github.com/grafana/oncall/pull/2537))

## v1.3.12 (2023-07-14)

Expand Down
5 changes: 2 additions & 3 deletions engine/apps/alerts/paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def _trigger_alert(
deleted_at=None,
defaults={
"author": from_user,
"verbal_name": "Direct paging",
"verbal_name": f"Direct paging ({team.name if team else 'No'} team)",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to return team name to integration name, this way it's easier to filter by integration on the alert groups page.
It also makes things more consistent with other platforms where it's not possible to provide a team widget alongside integration name (e.g. Slack).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure it is a good practice to hardcode it lake that. We should use the value from teams table in the places you mentioned

iskhakov marked this conversation as resolved.
Show resolved Hide resolved
},
)
if alert_receive_channel.default_channel_filter is None:
Expand Down Expand Up @@ -90,7 +90,7 @@ def _trigger_alert(
return alert.group


def check_user_availability(user: User, team: Team) -> list[dict[str, Any]]:
def check_user_availability(user: User) -> list[dict[str, Any]]:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made so that user availability doesn't depend on the selected team. My reasoning is the following:

  • Imagine this scenario: I want to page some team + some other person
  • This other person is probably from some other team, because why would I want to page two people from the same team? My assumption here is that most of the times when I want to notify additional responders, I want these responders to be from a different team (e.g. for alert groups relevant for multiple teams).

So I'd think it makes sense to check availability across all teams, not only the selected one.

"""Check user availability to be paged.

Return a warnings list indicating `error` and any additional related `data`.
Expand All @@ -108,7 +108,6 @@ def check_user_availability(user: User, team: Team) -> list[dict[str, Any]]:
schedules = OnCallSchedule.objects.filter(
Q(cached_ical_file_primary__contains=user.username) | Q(cached_ical_file_primary__contains=user.email),
organization=user.organization,
team=team,
)
schedules_data = {}
for s in schedules:
Expand Down
34 changes: 3 additions & 31 deletions engine/apps/alerts/tests/test_paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_check_user_availability_no_policies(make_organization, make_user_for_or
organization = make_organization()
user = make_user_for_organization(organization)

warnings = check_user_availability(user, None)
warnings = check_user_availability(user)
assert warnings == [
{"data": {}, "error": USER_HAS_NO_NOTIFICATION_POLICY},
{"data": {"schedules": {}}, "error": USER_IS_NOT_ON_CALL},
Expand All @@ -95,40 +95,12 @@ def test_check_user_availability_not_on_call(
make_schedule, make_on_call_shift, organization, None, other_user, extra_users=[user]
)

warnings = check_user_availability(user, None)
warnings = check_user_availability(user)
assert warnings == [
{"data": {"schedules": {schedule.name: {other_user.public_primary_key}}}, "error": USER_IS_NOT_ON_CALL},
]


@pytest.mark.django_db
def test_check_user_availability_on_call_different_team(
make_organization,
make_team,
make_user_for_organization,
make_user_notification_policy,
make_schedule,
make_on_call_shift,
):
organization = make_organization()
some_team = make_team(organization)
user = make_user_for_organization(organization)
make_user_notification_policy(
user=user,
step=UserNotificationPolicy.Step.NOTIFY,
notify_by=UserNotificationPolicy.NotificationChannel.SMS,
)

# setup on call schedule
# user is on call, but on a different team
setup_always_on_call_schedule(make_schedule, make_on_call_shift, organization, some_team, user)

warnings = check_user_availability(user, None)
assert warnings == [
{"data": {"schedules": {}}, "error": USER_IS_NOT_ON_CALL},
]


@pytest.mark.django_db
def test_check_user_availability_on_call(
make_organization,
Expand All @@ -150,7 +122,7 @@ def test_check_user_availability_on_call(
# setup on call schedule
setup_always_on_call_schedule(make_schedule, make_on_call_shift, organization, some_team, user)

warnings = check_user_availability(user, some_team)
warnings = check_user_availability(user)
assert warnings == []


Expand Down
2 changes: 1 addition & 1 deletion engine/apps/api/serializers/paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class DirectPagingSerializer(serializers.Serializer):
alert_group = serializers.HiddenField(default=None) # set in DirectPagingSerializer.validate

title = serializers.CharField(required=False, default=None)
message = serializers.CharField(required=False, default=None)
message = serializers.CharField(required=False, default=None, allow_null=True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frontend treats an empty message as null, updating the backend to support this.


team = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault())

Expand Down
2 changes: 1 addition & 1 deletion engine/apps/api/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ def export_token(self, request, pk):
@action(detail=True, methods=["get"])
def check_availability(self, request, pk):
user = self.get_object()
warnings = check_user_availability(user=user, team=request.user.current_team)
warnings = check_user_availability(user=user)
return Response(data={"warnings": warnings}, status=status.HTTP_200_OK)


Expand Down
15 changes: 14 additions & 1 deletion engine/apps/slack/scenarios/manual_incident.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
class StartCreateIncidentFromSlashCommand(scenario_step.ScenarioStep):
"""
StartCreateIncidentFromSlashCommand triggers creation of a manual incident from the slack message via slash command
THIS FEATURE IS DEPRECATED AND WILL BE REMOVED IN A FUTURE RELEASE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecating /oncall Slack command, so the only way to create alerts is /escalate

"""

command_name = [settings.SLACK_SLASH_COMMAND_NAME]
Expand Down Expand Up @@ -232,6 +233,18 @@ def process_scenario(self, slack_user_identity, slack_team_identity, payload):


def _get_manual_incident_form_view(routing_uid, blocks, private_metatada):
deprecation_blocks = [
{
"type": "header",
"text": {
"type": "plain_text",
"text": f":no_entry: This command is deprecated and will be removed soon. Please use {settings.SLACK_DIRECT_PAGING_SLASH_COMMAND} command instead :no_entry:",
"emoji": True,
},
},
{"type": "divider"},
]

view = {
"type": "modal",
"callback_id": routing_uid,
Expand All @@ -248,7 +261,7 @@ def _get_manual_incident_form_view(routing_uid, blocks, private_metatada):
"type": "plain_text",
"text": "Submit",
},
"blocks": blocks,
"blocks": deprecation_blocks + blocks,
"private_metadata": private_metatada,
}

Expand Down
Loading