Skip to content

Commit

Permalink
Add responders improvements (#3128)
Browse files Browse the repository at this point in the history
# What this PR does

https://www.loom.com/share/c5e10b5ec51343d0954c6f41cfd6a5fb

## Summary of backend changes
- Add `AlertReceiveChannel.get_orgs_direct_paging_integrations` method
and `AlertReceiveChannel.is_contactable` property. These are needed to
be able to (optionally) filter down teams, in the `GET /teams` internal
API endpoint
([here](https://github.com/grafana/oncall/pull/3128/files#diff-a4bd76e557f7e11dafb28a52c1034c075028c693b3c12d702d53c07fc6f24c05R55-R63)),
to just teams that have a "contactable" Direct Paging integration
- `engine/apps/alerts/paging.py`
- update these functions to support new UX. In short `direct_paging` no
longer takes a list of `ScheduleNotifications` or an `EscalationChain`
object
  - add `user_is_oncall` helper function
- add `_construct_title` helper function. In short if no `title` is
provided, which is the case for Direct Pages originating from OnCall
(either UI or Slack), then the format is `f"{from_user.username} is
paging <team.name (if team is specified> <comma separated list of
user.usernames> to join escalation"`
- `engine/apps/api/serializers/team.py` - add
`number_of_users_currently_oncall` attribute to response schema
([code](https://github.com/grafana/oncall/pull/3128/files#diff-26af48f796c9e987a76447586dd0f92349783d6ea6a0b6039a2f0f28bd58c2ebR45-R52))
- `engine/apps/api/serializers/user.py` - add `is_currently_oncall`
attribute to response schema
([code](https://github.com/grafana/oncall/pull/3128/files#diff-6744b5544ebb120437af98a996da5ad7d48ee1139a6112c7e3904010ab98f232R157-R162))
- `engine/apps/api/views/team.py` - add support for two new optional
query params `only_include_notifiable_teams` and `include_no_team`
([code](https://github.com/grafana/oncall/pull/3128/files#diff-a4bd76e557f7e11dafb28a52c1034c075028c693b3c12d702d53c07fc6f24c05R55-R70))
- `engine/apps/api/views/user.py`
- in the `GET /users` internal API endpoint, when specifying the
`search` query param now also search on `teams__name`
([code](https://github.com/grafana/oncall/pull/3128/files#diff-30309629484ad28e6fe09816e1bd226226d652ea977b6f3b6775976c729bf4b5R223);
this is a new UX requirement)
- add support for a new optional query param, `is_currently_oncall`, to
allow filtering users based on.. whether they are currently on call or
not
([code](https://github.com/grafana/oncall/pull/3128/files#diff-30309629484ad28e6fe09816e1bd226226d652ea977b6f3b6775976c729bf4b5R272-R282))
- remove `check_availability` endpoint (no longer used with new UX; also
removed references in frontend code)
- `engine/apps/slack/scenarios/paging.py` and
`engine/apps/slack/scenarios/manage_responders.py` - update Slack
workflows to support new UX. Schedules are no longer a concept here.
When creating a new alert group via `/escalate` the user either
specifies a team and/or user(s) (they must specify at least one of the
two and validation is done here to check this). When adding responders
to an existing alert group it's simply a list of users that they can
add, no more schedules.
- add `Organization.slack_is_configured` and
`Organization.telegram_is_configured` properties. These are needed to
support [this new functionality
](https://github.com/grafana/oncall/pull/3128/files#diff-9d96504027309f2bd1e95352bac1433b09b60eb4fafb611b52a6c15ed16cbc48R271-R272)
in the `AlertReceiveChannel` model.

## Summary of frontend changes
- Refactor/rename `EscalationVariants` component to `AddResponders` +
remove `grafana-plugin/src/containers/UserWarningModal` (no longer
needed with new UX)
- Remove `grafana-plugin/src/models/user.ts` as it seemed to be a
duplicate of `grafana-plugin/src/models/user/user.types.ts`

Related to grafana/incident#4278

- Closes #3115
- Closes #3116
- Closes #3117
- Closes #3118 
- Closes #3177 

## TODO
- [x] make frontend changes
- [x] update Slack backend functionality
- [x] update public documentation
- [x] add/update e2e tests

## Post-deploy To-dos
- [ ] update dev/ops/production Slack bots to update `/escalate` command
description (should now say "Direct page a team or user(s)")

## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
  • Loading branch information
joeyorlando authored Oct 27, 2023
1 parent 685b517 commit 5093eee
Show file tree
Hide file tree
Showing 51 changed files with 3,036 additions and 1,395 deletions.
36 changes: 20 additions & 16 deletions e2e-tests/alerts/directPaging.test.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
import { test } from '../fixtures';
import { clickButton, fillInInput, selectDropdownValue } from '../utils/forms';
import { goToOnCallPage } from "../utils/navigation";
import { verifyAlertGroupTitleAndMessageContainText } from "../utils/alertGroup";
import { test, expect } from '../fixtures';
import { clickButton, fillInInput } from '../utils/forms';
import { goToOnCallPage } from '../utils/navigation';

test('we can create an alert group for default team', async ({ adminRolePage }) => {
/**
* TODO: test that we can also direct page a team. This is a bit more involved because we need to
* create a team via the Grafana API then go and configure the team's direct paging integration so that
* it will show up in the dropdown (ie. create an escalation chain and assign it to the integration)
*/

test('we can directly page a user', async ({ adminRolePage }) => {
const message = 'Help me please!';
const { page } = adminRolePage;

await goToOnCallPage(page, 'alert-groups');
await clickButton({ page, buttonText: 'New alert group' });
await clickButton({ page, buttonText: 'Escalation' });

await fillInInput(page, 'textarea[name="message"]', message);
await clickButton({ page, buttonText: 'Invite' });

await fillInInput(page, 'input[name="title"]', "Help me!");
await fillInInput(page, 'textarea[name="message"]', "Help me please!");
const addRespondersPopup = page.getByTestId('add-responders-popup');

await selectDropdownValue({
page,
selectType: 'grafanaSelect',
placeholderText: "Select team",
value: "No team",
});
await addRespondersPopup.getByText('Users').click();
await addRespondersPopup.getByText(adminRolePage.userName).click();

await clickButton({ page, buttonText: 'Create' });

// Check we are redirected to the alert group page
await page.waitForURL('**/alert-groups/I*'); // Alert group IDs always start with "I"
await verifyAlertGroupTitleAndMessageContainText(page, "Help me!", "Help me please!")
await page.waitForURL('**/alert-groups/I*'); // Alert group IDs always start with "I"
await expect(page.getByTestId('incident-message')).toContainText(message);
});
10 changes: 0 additions & 10 deletions e2e-tests/utils/alertGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,3 @@ export const verifyThatAlertGroupIsTriggered = async (

expect(await incidentTimelineContainsStep(page, triggeredStepText)).toBe(true);
};


export const verifyAlertGroupTitleAndMessageContainText = async (
page: Page,
title: string,
message: string
): Promise<void> => {
await expect(page.getByTestId('incident-title')).toContainText(title);
await expect(page.getByTestId('incident-message')).toContainText(message);
};
16 changes: 13 additions & 3 deletions src/components/GForm/GForm.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react';

import { Field, Form, Input, InputControl, Select, Switch, TextArea } from '@grafana/ui';
import { Field, Form, FormFieldErrors, Input, InputControl, Select, Switch, TextArea } from '@grafana/ui';
import { capitalCase } from 'change-case';
import cn from 'classnames/bind';
import { isEmpty } from 'lodash-es';

import Collapse from 'components/Collapse/Collapse';
import { FormItem, FormItemType } from 'components/GForm/GForm.types';
Expand All @@ -20,6 +21,7 @@ interface GFormProps {
form: { name: string; fields: FormItem[] };
data: any;
onSubmit: (data: any) => void;
onChange?: (formIsValid: boolean) => void;

customFieldSectionRenderer?: React.FC<CustomFieldSectionRendererProps>;
onFieldRender?: (
Expand Down Expand Up @@ -190,7 +192,13 @@ class GForm extends React.Component<GFormProps, {}> {
? formItem.getDisabled(getValues())
: false;

const formControl = renderFormControl(formItem, register, control, disabled, this.onChange);
const formControl = renderFormControl(
formItem,
register,
control,
disabled,
this.onChange.bind(this, errors)
);

if (CustomFieldSectionRenderer && formItem.type === FormItemType.Other && formItem.render) {
return (
Expand Down Expand Up @@ -245,7 +253,9 @@ class GForm extends React.Component<GFormProps, {}> {
);
}

onChange = (field: any, value: string) => {
onChange = (errors: FormFieldErrors, field: any, value: string) => {
this.props.onChange?.(isEmpty(errors));

field?.onChange(value);
this.forceUpdate();
};
Expand Down
7 changes: 4 additions & 3 deletions src/components/GTable/GTable.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React, { FC, useCallback, useMemo, ChangeEvent } from 'react';
import React, { useCallback, useMemo, ChangeEvent, ReactElement } from 'react';

import { Pagination, Checkbox, Icon } from '@grafana/ui';
import cn from 'classnames/bind';
import Table from 'rc-table';
import { TableProps } from 'rc-table/lib/Table';
import { DefaultRecordType } from 'rc-table/lib/interface';

import styles from './GTable.module.css';

Expand Down Expand Up @@ -31,7 +32,7 @@ export interface Props<RecordType = unknown> extends TableProps<RecordType> {
showHeader?: boolean;
}

const GTable: FC<Props> = (props) => {
const GTable = <RT extends DefaultRecordType = DefaultRecordType>(props: Props<RT>): ReactElement => {
const {
columns: columnsProp,
data,
Expand Down Expand Up @@ -139,7 +140,7 @@ const GTable: FC<Props> = (props) => {

return (
<div className={cx('root')} data-testid="test__gTable">
<Table
<Table<RT>
expandable={expandable}
rowKey={rowKey}
className={cx('filter-table', className)}
Expand Down
14 changes: 6 additions & 8 deletions src/components/ManualAlertGroup/ManualAlertGroup.config.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
import { FormItem, FormItemType } from 'components/GForm/GForm.types';

export type ManualAlertGroupFormData = {
message: string;
};

export const manualAlertFormConfig: { name: string; fields: FormItem[] } = {
name: 'Manual Alert Group',
fields: [
{
name: 'title',
type: FormItemType.Input,
label: 'Title',
validation: { required: true },
},
{
name: 'message',
type: FormItemType.TextArea,
label: 'Message (optional)',
validation: { required: false },
label: 'What is going on?',
validation: { required: true },
},
],
};
16 changes: 0 additions & 16 deletions src/components/ManualAlertGroup/ManualAlertGroup.module.css

This file was deleted.

Loading

0 comments on commit 5093eee

Please sign in to comment.