From 96f9de6299217d642f4b3342b1f4461ef9051582 Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Fri, 27 Jan 2023 17:40:41 +0300 Subject: [PATCH 01/16] fix NPE in schedule user details (#1230) # What this PR does Fix NPE ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/1229 ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --------- Co-authored-by: Vadim Stepanov --- CHANGELOG.md | 1 + .../components/ScheduleUserDetails/ScheduleUserDetails.tsx | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1790de..42865e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix bugs related to creating contact point for Grafana Alerting integration +- Fixed NPE in ScheduleUserDetails component ([#1229](https://github.com/grafana/oncall/issues/1229)) ## v1.1.19 (2023-01-25) diff --git a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx index ee0164d1..1f09dcc2 100644 --- a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx +++ b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx @@ -30,8 +30,8 @@ const ScheduleUserDetails: FC = (props) => { const store = useStore(); const { teamStore } = store; - let slackWorkspaceNameOrigin = teamStore.currentTeam.slack_team_identity.cached_name; - const slackWorkspaceName = slackWorkspaceNameOrigin.replace(/[^0-9a-z]/gi, ''); + + const slackWorkspaceName = teamStore.currentTeam.slack_team_identity?.cached_name?.replace(/[^0-9a-z]/gi, '') || ''; return (
From e0ae9919c7c5168c935c63b5679480c34e7d211f Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 27 Jan 2023 14:10:44 -0300 Subject: [PATCH 02/16] Add paging for direct paging users in slack dialog (#1232) Fixes issue when there are more than 100 users to be listed in the direct pagination responders select. Alternatively we should consider moving to an `external_select` block later. --- engine/apps/slack/scenarios/paging.py | 41 +++++++++++++++++++-------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index 6f25e859..15ec4030 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -559,20 +559,37 @@ def _get_users_select(organization, team, input_id_prefix): } for user in users ] + if not user_options: - user_select = {"type": "context", "elements": [{"type": "mrkdwn", "text": "No users available"}]} + return {"type": "context", "elements": [{"type": "mrkdwn", "text": "No users available"}]} + + user_select = { + "type": "section", + "text": {"type": "mrkdwn", "text": "Add responders"}, + "block_id": input_id_prefix + DIRECT_PAGING_USER_SELECT_ID, + "accessory": { + "type": "static_select", + "placeholder": {"type": "plain_text", "text": "Select a user", "emoji": True}, + "action_id": OnPagingUserChange.routing_uid(), + }, + } + + if len(user_options) > scenario_step.MAX_STATIC_SELECT_OPTIONS: + # paginate user options in groups + max_length = scenario_step.MAX_STATIC_SELECT_OPTIONS + chunks = [user_options[x : x + max_length] for x in range(0, len(user_options), max_length)] + option_groups = [ + { + "label": {"type": "plain_text", "text": f"({(i * max_length)+1}-{(i * max_length)+max_length})"}, + "options": group, + } + for i, group in enumerate(chunks) + ] + user_select["accessory"]["option_groups"] = option_groups + else: - user_select = { - "type": "section", - "text": {"type": "mrkdwn", "text": "Add responders"}, - "block_id": input_id_prefix + DIRECT_PAGING_USER_SELECT_ID, - "accessory": { - "type": "static_select", - "placeholder": {"type": "plain_text", "text": "Select a user", "emoji": True}, - "options": user_options, - "action_id": OnPagingUserChange.routing_uid(), - }, - } + user_select["accessory"]["options"] = user_options + return user_select From ae44ee56521bae5a7b38da5594733555205b3757 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Sat, 28 Jan 2023 12:50:41 +0800 Subject: [PATCH 03/16] Cache render_for_web field for alertgroups list serializer (#1236) # What this PR does This PR caches the field `render_for_web` with lifetime 1 day and cache becomes invalid if it was created before * last alert received * template changed ## Which issue(s) this PR fixes ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --- ...eceivechannel_web_templates_modified_at.py | 18 +++++++++++ .../alerts/models/alert_receive_channel.py | 1 + engine/apps/api/serializers/alert_group.py | 30 +++++++++++++++++-- .../api/serializers/alert_receive_channel.py | 4 +++ 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py diff --git a/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py b/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py new file mode 100644 index 00000000..690f9e4b --- /dev/null +++ b/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.16 on 2023-01-27 07:41 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0008_alter_alertgrouplogrecord_type'), + ] + + operations = [ + migrations.AddField( + model_name='alertreceivechannel', + name='web_templates_modified_at', + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 076d9469..edafbf34 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -160,6 +160,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): web_title_template = models.TextField(null=True, default=None) web_message_template = models.TextField(null=True, default=None) web_image_url_template = models.TextField(null=True, default=None) + web_templates_modified_at = models.DateTimeField(blank=True, null=True) # email related fields are deprecated in favour of messaging backend based templates # these templates are stored in the messaging_backends_templates field diff --git a/engine/apps/api/serializers/alert_group.py b/engine/apps/api/serializers/alert_group.py index f25cd39b..5f3c854e 100644 --- a/engine/apps/api/serializers/alert_group.py +++ b/engine/apps/api/serializers/alert_group.py @@ -1,5 +1,7 @@ import logging +from django.core.cache import cache +from django.utils import timezone from rest_framework import serializers from apps.alerts.incident_appearance.renderers.classic_markdown_renderer import AlertGroupClassicMarkdownRenderer @@ -13,6 +15,7 @@ from .alert_receive_channel import FastAlertReceiveChannelSerializer from .user import FastUserSerializer logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) class ShortAlertGroupSerializer(serializers.ModelSerializer): @@ -92,10 +95,33 @@ class AlertGroupListSerializer(EagerLoadingMixin, serializers.ModelSerializer): if not obj.last_alert: return {} - return AlertGroupWebRenderer(obj, obj.last_alert).render() + web_templates_modified_at = obj.channel.web_templates_modified_at + last_alert_created_at = obj.last_alert.created_at + + CACHE_KEY = f"render_for_web_alert_group_{obj.id}" + CACHE_LIFEIME = 60 * 60 * 24 + cached_render_for_web = cache.get(CACHE_KEY, None) + + # use cache only if cache exists + # and cache was created after the last alert created + # and either web templates never modified + # or cache was created after templates were modified + if ( + cached_render_for_web is not None + and cached_render_for_web.get("cache_created_at") > last_alert_created_at + and ( + web_templates_modified_at is None + or cached_render_for_web.get("cache_created_at") > web_templates_modified_at + ) + ): + render_for_web = cached_render_for_web.get("render_for_web") + else: + render_for_web = AlertGroupWebRenderer(obj, obj.last_alert).render() + cache.set(CACHE_KEY, {"cache_created_at": timezone.now(), "render_for_web": render_for_web}, CACHE_LIFEIME) + + return render_for_web def get_render_for_classic_markdown(self, obj): - # alert group has no alerts if not obj.last_alert: return {} diff --git a/engine/apps/api/serializers/alert_receive_channel.py b/engine/apps/api/serializers/alert_receive_channel.py index 0df3d11d..b0e52c50 100644 --- a/engine/apps/api/serializers/alert_receive_channel.py +++ b/engine/apps/api/serializers/alert_receive_channel.py @@ -7,6 +7,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import URLValidator from django.template.loader import render_to_string +from django.utils import timezone from jinja2 import TemplateSyntaxError from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -379,6 +380,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_title_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_title_template = None + self.instance.web_templates_modified_at = timezone.now() def get_web_message_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_WEB_MESSAGE_TEMPLATE[obj.integration] @@ -390,6 +392,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_message_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_message_template = None + self.instance.web_templates_modified_at = timezone.now() def get_web_image_url_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_WEB_IMAGE_URL_TEMPLATE[obj.integration] @@ -401,6 +404,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_image_url_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_image_url_template = None + self.instance.web_templates_modified_at = timezone.now() def get_telegram_title_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_TELEGRAM_TITLE_TEMPLATE[obj.integration] From 6640a4092b70943d334441be093777d3e96dc6bd Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Sun, 29 Jan 2023 19:25:57 +0800 Subject: [PATCH 04/16] Create release.yml --- release.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 release.yml diff --git a/release.yml b/release.yml new file mode 100644 index 00000000..7cf738f1 --- /dev/null +++ b/release.yml @@ -0,0 +1,10 @@ +changelog: + exclude: + labels: + - ignore-for-release + authors: + - github-actions + categories: + - title: What's Changed + labels: + - "*" From a47fc7048e772fe2c076eba7b4a20c8534ca6a67 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Mon, 30 Jan 2023 11:10:10 +0800 Subject: [PATCH 05/16] Delete release.yml --- release.yml | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 release.yml diff --git a/release.yml b/release.yml deleted file mode 100644 index 7cf738f1..00000000 --- a/release.yml +++ /dev/null @@ -1,10 +0,0 @@ -changelog: - exclude: - labels: - - ignore-for-release - authors: - - github-actions - categories: - - title: What's Changed - labels: - - "*" From 63e91f896b1118de56783dba1223f1940682bd5b Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 30 Jan 2023 11:23:02 +0100 Subject: [PATCH 06/16] [RBAC] - minor UI bug fixes (#1185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does - remove hardcoded references to "Admin" and "Editor". Instead, use the `determineRequiredAuthString` and `generateMissingPermissionMessage` methods from `utils/authorization` which conditionally show the permission or role depending on whether RBAC is enabled or not - fix bug on list users page that always showed "Loading...". ![Screenshot 2023-01-21 at 09 29 35](https://user-images.githubusercontent.com/9406895/213859785-e9852838-5671-4275-aaed-4df75446ab6a.png) - only show users the user's table if they are allowed, otherwise show them this - RBAC enabled ![Screenshot 2023-01-26 at 09 09 57](https://user-images.githubusercontent.com/9406895/214786723-3389ce9c-7353-4216-9176-6547f2076660.png) - RBAC disabled ![Screenshot 2023-01-26 at 09 05 30](https://user-images.githubusercontent.com/9406895/214786739-eb9ee108-e79c-4105-912a-8bb5bf03cb32.png) - `Schedules Editor` role minor issue - Viewers are not allowed to list users by default, so granting schedule edit permission to them won’t allow them to see who rotations are assigned to or assign a rotation to a user. To make this a more straightforward user experience, instead of saying "No options found", we will tell the user that they are missing a particular role/permission to view these users: Before: ![Screenshot 2023-01-23 at 09 52 28](https://user-images.githubusercontent.com/9406895/213999896-d889540e-2835-4133-965a-306923a3e33b.png) After: ![Screenshot 2023-01-26 at 12 39 30](https://user-images.githubusercontent.com/9406895/214827179-d127002f-793e-4ab7-ad75-dfab653b7d7d.png) ## Which issue(s) this PR fixes - closes #971 ## Checklist - [x] Tests updated - [ ] Documentation added (N/A) - [x] `CHANGELOG.md` updated --- CHANGELOG.md | 2 + .../src/components/UserGroups/UserGroups.tsx | 2 + .../ApiTokenSettings/ApiTokenSettings.tsx | 20 ++++++---- .../containers/RemoteSelect/RemoteSelect.tsx | 36 +++++++++++------ .../containers/UserSettings/parts/index.tsx | 1 - grafana-plugin/src/network/index.ts | 2 + grafana-plugin/src/pages/index.tsx | 2 - .../pages/settings/tabs/Cloud/CloudPage.tsx | 20 ++++------ grafana-plugin/src/pages/users/Users.tsx | 39 +++++++++++-------- grafana-plugin/src/plugin.json | 4 +- .../plugin/__snapshots__/plugin.test.ts.snap | 8 ++-- grafana-plugin/src/state/plugin/index.ts | 11 +++--- .../src/state/plugin/plugin.test.ts | 39 ++++++++++++------- .../utils/authorization/authorization.test.ts | 26 +++++++++++++ .../src/utils/authorization/index.ts | 24 +++++++++--- 15 files changed, 152 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42865e21..72b86e13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix bugs related to creating contact point for Grafana Alerting integration +- Fix minor UI bug on OnCall users page where it would idefinitely show a "Loading..." message +- Only show OnCall user's table to users that are authorized - Fixed NPE in ScheduleUserDetails component ([#1229](https://github.com/grafana/oncall/issues/1229)) ## v1.1.19 (2023-01-25) diff --git a/grafana-plugin/src/components/UserGroups/UserGroups.tsx b/grafana-plugin/src/components/UserGroups/UserGroups.tsx index 5a664060..7dfb9dcc 100644 --- a/grafana-plugin/src/components/UserGroups/UserGroups.tsx +++ b/grafana-plugin/src/components/UserGroups/UserGroups.tsx @@ -8,6 +8,7 @@ import { SortableContainer, SortableElement, SortableHandle } from 'react-sortab import Text from 'components/Text/Text'; import RemoteSelect from 'containers/RemoteSelect/RemoteSelect'; import { User } from 'models/user/user.types'; +import { UserActions } from 'utils/authorization'; import { fromPlainArray, toPlainArray } from './UserGroups.helpers'; import { Item } from './UserGroups.types'; @@ -114,6 +115,7 @@ const UserGroups = (props: UserGroupsProps) => { onChange={handleUserAdd} showError={showError} maxMenuHeight={150} + requiredUserAction={UserActions.UserSettingsWrite} /> { }, ]; + const authorizedToViewAPIKeys = isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW); + + let emptyText = 'Loading...'; + if (!authorizedToViewAPIKeys) { + emptyText = `${generateMissingPermissionMessage(REQUIRED_PERMISSION_TO_VIEW)} to be able to view API tokens.`; + } else if (apiTokens) { + emptyText = 'No tokens found'; + } + return ( <> { className="api-keys" showHeader={!isMobile} data={apiTokens} - emptyText={ - isUserActionAllowed(UserActions.APIKeysWrite) - ? apiTokens - ? 'No tokens found' - : 'Loading...' - : 'API tokens are available only for users with Admin permissions' - } + emptyText={emptyText} columns={columns} /> {showCreateTokenModal && ( diff --git a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx index 753b3326..91b17d19 100644 --- a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx +++ b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx @@ -1,10 +1,11 @@ -import React, { useCallback, useEffect, useMemo, useReducer } from 'react'; +import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { AsyncMultiSelect, AsyncSelect } from '@grafana/ui'; import { inject, observer } from 'mobx-react'; -import { makeRequest } from 'network'; +import { makeRequest, isNetworkError } from 'network'; +import { UserAction, generateMissingPermissionMessage } from 'utils/authorization'; interface RemoteSelectProps { autoFocus?: boolean; @@ -24,6 +25,7 @@ interface RemoteSelectProps { getOptionLabel?: (item: SelectableValue) => React.ReactNode; showError?: boolean; maxMenuHeight?: number; + requiredUserAction?: UserAction; } const RemoteSelect = inject('store')( @@ -45,9 +47,12 @@ const RemoteSelect = inject('store')( openMenuOnFocus = true, showError, maxMenuHeight, + requiredUserAction, } = props; - const getOptions = (data: any[]) => { + const [noOptionsMessage, setNoOptionsMessage] = useState('No options found'); + + const getOptions = (data: any[]): SelectableValue[] => { return data.map((option: any) => ({ value: option[valueField], label: option[fieldToShow], @@ -62,17 +67,23 @@ const RemoteSelect = inject('store')( const [options, setOptions] = useReducer(mergeOptions, []); - useEffect(() => { - makeRequest(href, {}).then((data) => { - setOptions(getOptions(data.results || data)); - }); + const loadOptionsCallback = useCallback(async (query?: string): Promise => { + try { + const data = await makeRequest(href, { params: { search: query } }); + const options = getOptions(data.results || data); + setOptions(options); + + return options; + } catch (e) { + if (isNetworkError(e) && e.response.status === 403 && requiredUserAction) { + setNoOptionsMessage(generateMissingPermissionMessage(requiredUserAction)); + } + return []; + } }, []); - const loadOptionsCallback = useCallback((query: string) => { - return makeRequest(href, { params: { search: query } }).then((data) => { - setOptions(getOptions(data.results || data)); - return getOptions(data.results || data); - }); + useEffect(() => { + loadOptionsCallback(); }, []); const onChangeCallback = useCallback( @@ -119,6 +130,7 @@ const RemoteSelect = inject('store')( defaultOptions={options} loadOptions={loadOptionsCallback} getOptionLabel={getOptionLabel} + noOptionsMessage={noOptionsMessage} invalid={showError} /> ); diff --git a/grafana-plugin/src/containers/UserSettings/parts/index.tsx b/grafana-plugin/src/containers/UserSettings/parts/index.tsx index 77692b62..cf0eb994 100644 --- a/grafana-plugin/src/containers/UserSettings/parts/index.tsx +++ b/grafana-plugin/src/containers/UserSettings/parts/index.tsx @@ -133,7 +133,6 @@ export const TabsContent = observer(({ id, activeTab, onTabChange, isDesktopOrLa ) : ( ))} - {/* TODO: we should probably hide this tab when a user (ie. Admin) is viewing the user settings for another user. Would it make sense for an Admin to be able to link their mobile app to another user's profile */} {activeTab === UserSettingsTab.MobileAppConnection && } {activeTab === UserSettingsTab.SlackInfo && } {activeTab === UserSettingsTab.TelegramInfo && } diff --git a/grafana-plugin/src/network/index.ts b/grafana-plugin/src/network/index.ts index e9c1930e..53b1f579 100644 --- a/grafana-plugin/src/network/index.ts +++ b/grafana-plugin/src/network/index.ts @@ -34,6 +34,8 @@ interface RequestConfig { validateStatus?: (status: number) => boolean; } +export const isNetworkError = axios.isAxiosError; + export const makeRequest = async (path: string, config: RequestConfig) => { const { method = 'GET', params, data, validateStatus } = config; diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index 8bcb10e0..6de5109a 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -127,7 +127,6 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'table', id: 'live-settings', text: 'Env Variables', - role: 'Admin', hideFromTabsFn: (store: RootBaseStore) => { const hasLiveSettings = store.hasFeature(AppFeature.LiveSettings); return isTopNavbar() || !hasLiveSettings; @@ -139,7 +138,6 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'cloud', id: 'cloud', text: 'Cloud', - role: 'Admin', hideFromTabsFn: (store: RootBaseStore) => { const hasCloudFeature = store.hasFeature(AppFeature.CloudConnection); return isTopNavbar() || !hasCloudFeature; diff --git a/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx b/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx index 38777de6..6eb3a7cf 100644 --- a/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx +++ b/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx @@ -16,6 +16,7 @@ import { WithStoreProps } from 'state/types'; import { useStore } from 'state/useStore'; import { withMobXProviderContext } from 'state/withStore'; import { openErrorNotification } from 'utils'; +import { determineRequiredAuthString, UserActions } from 'utils/authorization'; import { PLUGIN_ROOT } from 'utils/consts'; import styles from './CloudPage.module.css'; @@ -261,10 +262,9 @@ const CloudPage = observer((props: CloudPageProps) => {
- {/* TODO: should probably update this message? */} - { - 'Ask your users to sign up in Grafana Cloud, verify phone number and feel free to set up SMS & phone call notifications in personal settings! Only users with Admin or Editor role will be synced.' - } + {`Ask your users to sign up in Grafana Cloud, verify phone number and feel free to set up SMS & phone call notifications in personal settings! Users must have ${determineRequiredAuthString( + UserActions.NotificationsRead + )} in order to be synced.`} { {matched_users_count === 1 ? '' : 's'} {` matched between OSS and Cloud OnCall`} - {syncingUsers ? ( - - ) : ( - - )} +
)} diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx index bbaff4da..19fa6853 100644 --- a/grafana-plugin/src/pages/users/Users.tsx +++ b/grafana-plugin/src/pages/users/Users.tsx @@ -23,7 +23,7 @@ import { User as UserType } from 'models/user/user.types'; import { PageProps, WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; import LocationHelper from 'utils/LocationHelper'; -import { isUserActionAllowed, UserActions } from 'utils/authorization'; +import { generateMissingPermissionMessage, isUserActionAllowed, UserActions } from 'utils/authorization'; import { PLUGIN_ROOT } from 'utils/consts'; import { getUserRowClassNameFn } from './Users.helpers'; @@ -35,6 +35,7 @@ const cx = cn.bind(styles); interface UsersProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} const ITEMS_PER_PAGE = 100; +const REQUIRED_PERMISSION_TO_VIEW_USERS = UserActions.UserSettingsWrite; interface UsersState extends PageBaseState { page: number; @@ -43,6 +44,7 @@ interface UsersState extends PageBaseState { usersFilters?: { searchTerm: string; }; + initialUsersLoaded: boolean; } @observer @@ -56,10 +58,9 @@ class Users extends React.Component { }, errorData: initErrorDataState(), + initialUsersLoaded: false, }; - initialUsersLoaded = false; - async componentDidMount() { const { query: { p }, @@ -74,18 +75,19 @@ class Users extends React.Component { const { usersFilters, page } = this.state; const { userStore } = store; - if (!isUserActionAllowed(UserActions.UserSettingsWrite)) { + if (!isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW_USERS)) { return; } LocationHelper.update({ p: page }, 'partial'); - return await userStore.updateItems(usersFilters, page); + await userStore.updateItems(usersFilters, page); + + this.setState({ initialUsersLoaded: true }); }; componentDidUpdate(prevProps: UsersProps) { - if (!this.initialUsersLoaded && isUserActionAllowed(UserActions.UserSettingsWrite)) { + if (!this.state.initialUsersLoaded) { this.updateUsers(); - this.initialUsersLoaded = true; } if (prevProps.match.params.id !== this.props.match.params.id) { @@ -117,7 +119,7 @@ class Users extends React.Component { }; render() { - const { usersFilters, userPkToEdit, page, errorData } = this.state; + const { usersFilters, userPkToEdit, page, errorData, initialUsersLoaded } = this.state; const { store, match: { @@ -165,6 +167,8 @@ class Users extends React.Component { const { count, results } = userStore.getSearchResult(); + const authorizedToViewUsers = isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW_USERS); + return ( { Users - - To manage permissions or add users, please visit{' '} - Grafana user management - + {authorizedToViewUsers && ( + + To manage permissions or add users, please visit{' '} + Grafana user management + + )}
@@ -194,7 +200,7 @@ class Users extends React.Component { - {isUserActionAllowed(UserActions.UserSettingsRead) ? ( + {authorizedToViewUsers ? ( <>
{
{ /* @ts-ignore */ title={ <> - You don't have enough permissions to view other users because you are not Admin.{' '} - Click here to open your profile + {generateMissingPermissionMessage(REQUIRED_PERMISSION_TO_VIEW_USERS)} to be able to view OnCall + users. Click here to open your + profile } severity="info" diff --git a/grafana-plugin/src/plugin.json b/grafana-plugin/src/plugin.json index 49790bce..efde7c9c 100644 --- a/grafana-plugin/src/plugin.json +++ b/grafana-plugin/src/plugin.json @@ -535,7 +535,7 @@ { "role": { "name": "User Settings Reader", - "description": "Read-only access to OnCall User Settings", + "description": "Read-only access to own OnCall User Settings", "permissions": [ { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" } @@ -546,7 +546,7 @@ { "role": { "name": "User Settings Editor", - "description": "Read/write access to own OnCall User Settings", + "description": "Read/write access to own OnCall User Settings + ability to view basic information about other OnCall users", "permissions": [ { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" }, diff --git a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap index 334e4c70..1bb513e3 100644 --- a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap +++ b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap @@ -34,19 +34,19 @@ exports[`PluginState.generateUnknownErrorMsg it returns the proper error message Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 AxiosError properly - has custom error message: false 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: false 1`] = ` "An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 AxiosError properly - has custom error message: true 1`] = `"ohhhh nooo an error"`; +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: true 1`] = `"ohhhh nooo an error"`; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 AxiosError properly - status code: 409 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 409 1`] = ` "An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 AxiosError properly - status code: 502 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 502 1`] = ` "Could not communicate with your OnCall API at http://hello.com (NOTE: your OnCall API URL is currently being taken from process.env of your UI). Validate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance." `; diff --git a/grafana-plugin/src/state/plugin/index.ts b/grafana-plugin/src/state/plugin/index.ts index 454fee84..f0617bc9 100644 --- a/grafana-plugin/src/state/plugin/index.ts +++ b/grafana-plugin/src/state/plugin/index.ts @@ -1,8 +1,7 @@ import { getBackendSrv } from '@grafana/runtime'; -import axios from 'axios'; import { OnCallAppPluginMeta, OnCallPluginMetaJSONData, OnCallPluginMetaSecureJSONData } from 'types'; -import { makeRequest } from 'network'; +import { makeRequest, isNetworkError } from 'network'; import FaroHelper from 'utils/faro'; export type UpdateGrafanaPluginSettingsProps = { @@ -76,7 +75,7 @@ class PluginState { ); const consoleMsg = `occured while trying to ${installationVerb} the plugin w/ the OnCall backend`; - if (axios.isAxiosError(e)) { + if (isNetworkError(e)) { const { status: statusCode } = e.response; console.warn(`An HTTP related error ${consoleMsg}`, e.response); @@ -100,7 +99,7 @@ class PluginState { errorMsg = unknownErrorMsg; } } else { - // a non-axios related error occured.. this scenario shouldn't occur... + // a non-network related error occured.. this scenario shouldn't occur... console.warn(`An unknown error ${consoleMsg}`, e); errorMsg = unknownErrorMsg; } @@ -115,12 +114,12 @@ class PluginState { ): string => { let errorMsg: string; - if (axios.isAxiosError(e)) { + if (isNetworkError(e)) { // The user likely put in a bogus URL for the OnCall API URL console.warn('An HTTP related error occured while trying to provision the plugin w/ Grafana', e.response); errorMsg = this.generateInvalidOnCallApiURLErrorMsg(onCallApiUrl, onCallApiUrlIsConfiguredThroughEnvVar); } else { - // a non-axios related error occured.. this scenario shouldn't occur... + // a non-network related error occured.. this scenario shouldn't occur... console.warn('An unknown error occured while trying to provision the plugin w/ Grafana', e); errorMsg = this.generateUnknownErrorMsg(onCallApiUrl, installationVerb, onCallApiUrlIsConfiguredThroughEnvVar); } diff --git a/grafana-plugin/src/state/plugin/plugin.test.ts b/grafana-plugin/src/state/plugin/plugin.test.ts index 405d9913..c18ab7b8 100644 --- a/grafana-plugin/src/state/plugin/plugin.test.ts +++ b/grafana-plugin/src/state/plugin/plugin.test.ts @@ -1,8 +1,9 @@ -import { makeRequest as makeRequestOriginal } from 'network'; +import { makeRequest as makeRequestOriginal, isNetworkError as isNetworkErrorOriginal } from 'network'; import PluginState, { InstallationVerb, PluginSyncStatusResponse, UpdateGrafanaPluginSettingsProps } from './'; const makeRequest = makeRequestOriginal as jest.Mock>; +const isNetworkError = isNetworkErrorOriginal as unknown as jest.Mock>; jest.mock('network'); @@ -13,7 +14,7 @@ afterEach(() => { const ONCALL_BASE_URL = '/plugin'; const GRAFANA_PLUGIN_SETTINGS_URL = '/api/plugins/grafana-oncall-app/settings'; -const generateMockAxiosError = (status: number, data = {}) => ({ isAxiosError: true, response: { status, ...data } }); +const generateMockNetworkError = (status: number, data = {}) => ({ response: { status, ...data } }); describe('PluginState.generateOnCallApiUrlConfiguredThroughEnvVarMsg', () => { test.each([true, false])( @@ -54,10 +55,12 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { console.warn = () => {}; }); - test.each([502, 409])('it handles a non-400 AxiosError properly - status code: %s', (status) => { + test.each([502, 409])('it handles a non-400 network error properly - status code: %s', (status) => { + isNetworkError.mockReturnValueOnce(true); + expect( PluginState.getHumanReadableErrorFromOnCallError( - generateMockAxiosError(status), + generateMockNetworkError(status), 'http://hello.com', 'install', true @@ -66,19 +69,23 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { }); test.each([true, false])( - 'it handles a 400 AxiosError properly - has custom error message: %s', + 'it handles a 400 network error properly - has custom error message: %s', (hasCustomErrorMessage) => { - const axiosError = generateMockAxiosError(400) as any; + isNetworkError.mockReturnValueOnce(true); + + const networkError = generateMockNetworkError(400) as any; if (hasCustomErrorMessage) { - axiosError.response.data = { error: 'ohhhh nooo an error' }; + networkError.response.data = { error: 'ohhhh nooo an error' }; } expect( - PluginState.getHumanReadableErrorFromOnCallError(axiosError, 'http://hello.com', 'install', true) + PluginState.getHumanReadableErrorFromOnCallError(networkError, 'http://hello.com', 'install', true) ).toMatchSnapshot(); } ); test('it handles an unknown error properly', () => { + isNetworkError.mockReturnValueOnce(false); + expect( PluginState.getHumanReadableErrorFromOnCallError(new Error('asdfasdf'), 'http://hello.com', 'install', true) ).toMatchSnapshot(); @@ -86,23 +93,27 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { }); describe('PluginState.getHumanReadableErrorFromGrafanaProvisioningError', () => { - test.each([true, false])('it handles an error properly', (isAxiosError) => { + beforeEach(() => { + console.warn = () => {}; + }); + + test.each([true, false])('it handles an error properly - network error: %s', (networkError) => { const onCallApiUrl = 'http://hello.com'; const installationVerb = 'install'; const onCallApiUrlIsConfiguredThroughEnvVar = true; - const axiosError = generateMockAxiosError(400); - const nonAxiosError = new Error('oh noooo'); - const error = isAxiosError ? axiosError : nonAxiosError; + const error = networkError ? generateMockNetworkError(400) : new Error('oh noooo'); const mockGenerateInvalidOnCallApiURLErrorMsgResult = 'asdadslkjfkjlsd'; const mockGenerateUnknownErrorMsgResult = 'asdadslkjfkjlsd'; + isNetworkError.mockReturnValueOnce(networkError); + PluginState.generateInvalidOnCallApiURLErrorMsg = jest .fn() .mockReturnValueOnce(mockGenerateInvalidOnCallApiURLErrorMsgResult); PluginState.generateUnknownErrorMsg = jest.fn().mockReturnValueOnce(mockGenerateUnknownErrorMsgResult); - const expectedErrorMsg = isAxiosError + const expectedErrorMsg = networkError ? mockGenerateInvalidOnCallApiURLErrorMsgResult : mockGenerateUnknownErrorMsgResult; @@ -115,7 +126,7 @@ describe('PluginState.getHumanReadableErrorFromGrafanaProvisioningError', () => ) ).toEqual(expectedErrorMsg); - if (isAxiosError) { + if (networkError) { expect(PluginState.generateInvalidOnCallApiURLErrorMsg).toHaveBeenCalledTimes(1); expect(PluginState.generateInvalidOnCallApiURLErrorMsg).toHaveBeenCalledWith( onCallApiUrl, diff --git a/grafana-plugin/src/utils/authorization/authorization.test.ts b/grafana-plugin/src/utils/authorization/authorization.test.ts index b0c72d78..dc2dae38 100644 --- a/grafana-plugin/src/utils/authorization/authorization.test.ts +++ b/grafana-plugin/src/utils/authorization/authorization.test.ts @@ -63,6 +63,32 @@ describe('isUserActionAllowed', () => { }); }); +describe('determineRequiredAuthString', () => { + const testPerm = auth.UserActions.UserSettingsRead; + + test.each([ + [true, `${testPerm.permission} permission`], + [false, `${testPerm.fallbackMinimumRoleRequired} role`], + ])('RBAC enabled: %s', (rbacEnabled, expected) => { + config.featureToggles.accessControlOnCall = rbacEnabled; + + expect(auth.determineRequiredAuthString(testPerm)).toBe(expected); + }); +}); + +describe('generateMissingPermissionMessage', () => { + const testPerm = auth.UserActions.UserSettingsRead; + + test.each([ + [true, `You are missing the ${testPerm.permission} permission`], + [false, `You are missing the ${testPerm.fallbackMinimumRoleRequired} role`], + ])('RBAC enabled: %s', (rbacEnabled, expected) => { + config.featureToggles.accessControlOnCall = rbacEnabled; + + expect(auth.generateMissingPermissionMessage(testPerm)).toBe(expected); + }); +}); + describe('generatePermissionString', () => { test('it properly builds permission strings with prefixes', () => { expect(auth.generatePermissionString(auth.Resource.API_KEYS, auth.Action.READ, true)).toEqual( diff --git a/grafana-plugin/src/utils/authorization/index.ts b/grafana-plugin/src/utils/authorization/index.ts index 215f58b2..10574c47 100644 --- a/grafana-plugin/src/utils/authorization/index.ts +++ b/grafana-plugin/src/utils/authorization/index.ts @@ -90,12 +90,24 @@ export const userHasMinimumRequiredRole = (minimumRoleRequired: OrgRole): boolea * * As a fallback (second argument), for cases where RBAC is not enabled for a grafana instance, rely on basic roles */ -export const isUserActionAllowed = ({ permission, fallbackMinimumRoleRequired }: UserAction): boolean => { - if (config.featureToggles.accessControlOnCall) { - return !!contextSrv.user.permissions?.[permission]; - } - return userHasMinimumRequiredRole(fallbackMinimumRoleRequired); -}; +export const isUserActionAllowed = ({ permission, fallbackMinimumRoleRequired }: UserAction): boolean => + config.featureToggles.accessControlOnCall + ? !!contextSrv.user.permissions?.[permission] + : userHasMinimumRequiredRole(fallbackMinimumRoleRequired); + +/** + * Given a `UserAction`, returns the permission or fallback-role, prefixed with "permission" or "role" respectively + * depending on whether or not RBAC is enabled/disabled + */ +export const determineRequiredAuthString = ({ permission, fallbackMinimumRoleRequired }: UserAction): string => + config.featureToggles.accessControlOnCall ? `${permission} permission` : `${fallbackMinimumRoleRequired} role`; + +/** + * Can be used to generate a user-friendly message about which permission is missing. Method is RBAC-aware + * and shows user the missing permission/basic-role depending on whether or not RBAC is enabled/disabled + */ +export const generateMissingPermissionMessage = (permission: UserAction): string => + `You are missing the ${determineRequiredAuthString(permission)}`; export const generatePermissionString = (resource: Resource, action: Action, includePrefix: boolean): string => `${includePrefix ? `${ONCALL_PERMISSION_PREFIX}.` : ''}${resource}:${action}`; From 9421ae25be5824bc724307c51018ed790e87ea91 Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Mon, 30 Jan 2023 13:34:35 +0300 Subject: [PATCH 07/16] Add Server URL below QR code for OSS for debugging purposes (#1209) # What this PR does ## Which issue(s) this PR fixes ## Checklist - [ ] Tests updated - [ ] Documentation added - [x] `CHANGELOG.md` updated --------- Co-authored-by: Joey Orlando Co-authored-by: Joey Orlando Co-authored-by: teodosii --- CHANGELOG.md | 1 + grafana-plugin/package.json | 1 + .../MobileAppConnection.tsx | 20 +++++++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72b86e13..86f5fde1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add Server URL below QR code for OSS for debugging purposes - Add Slack slash command allowing to trigger a direct page via a manually created alert group - Remove resolved and acknowledged filters as we switched to status ([#1201](https://github.com/grafana/oncall/pull/1201)) - Add sync with grafana on /users and /teams api calls from terraform plugin diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 63b9c244..da6da4de 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -10,6 +10,7 @@ "build": "grafana-toolkit plugin:build", "build:dev": "grafana-toolkit plugin:build --skipTest --skipLint", "test": "jest --verbose", + "test:silent": "jest --silent", "dev": "grafana-toolkit plugin:dev", "watch": "grafana-toolkit plugin:dev --watch", "sign": "grafana-toolkit plugin:sign", diff --git a/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.tsx b/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.tsx index 39de4a98..a7335139 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.tsx +++ b/grafana-plugin/src/containers/MobileAppConnection/MobileAppConnection.tsx @@ -12,6 +12,7 @@ import { User } from 'models/user/user.types'; import { AppFeature } from 'state/features'; import { useStore } from 'state/useStore'; import { isUserActionAllowed, UserActions } from 'utils/authorization'; +import { GRAFANA_LICENSE_OSS } from 'utils/consts'; import styles from './MobileAppConnection.module.scss'; import DisconnectButton from './parts/DisconnectButton/DisconnectButton'; @@ -153,6 +154,8 @@ const MobileAppConnection = observer(({ userPk }: Props) => { ); } else if (QRCodeValue) { + const QRCodeDataParsed = getParsedQRCodeValue(); + content = ( @@ -163,6 +166,15 @@ const MobileAppConnection = observer(({ userPk }: Props) => { {isQRBlurry && } + {store.backendLicense === GRAFANA_LICENSE_OSS && QRCodeDataParsed && ( + + Server URL embedded in this QR: +
+ + {QRCodeDataParsed.oncall_api_url} + + + )} ); } @@ -178,6 +190,14 @@ const MobileAppConnection = observer(({ userPk }: Props) => { ); + function getParsedQRCodeValue() { + try { + return JSON.parse(QRCodeValue); + } catch (ex) { + return undefined; + } + } + function clearTimeouts(): void { clearTimeout(userTimeoutId); clearTimeout(refreshTimeoutId); From 1eb96585418faeec3765c28c227253d0a7c11705 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 30 Jan 2023 11:43:15 +0100 Subject: [PATCH 08/16] fix failing lint github actions job due to issue w/ isort version (#1249) # What this PR does ## Which issue(s) this PR fixes `lint` github action jobs on all builds are failing right now ([example](https://github.com/grafana/oncall/actions/runs/4042567074/jobs/6950923821#step:6:16)) because of [this issue](https://github.com/PyCQA/isort/issues/2077) with `isort` ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7e427978..d86df0f6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pycqa/isort - rev: 5.9.3 + rev: 5.12.0 hooks: - id: isort files: ^engine From 32db4a1b7dca9b7f2701e358471fe1166255f429 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 30 Jan 2023 11:51:34 +0100 Subject: [PATCH 09/16] Bump simple-git from 3.15.0 to 3.16.0 in /grafana-plugin (#1233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [simple-git](https://github.com/steveukx/git-js/tree/HEAD/simple-git) from 3.15.0 to 3.16.0.
Release notes

Sourced from simple-git's releases.

simple-git@3.16.0

Minor Changes

  • 97fde2c: Support the use of -B in place of the default -b in checkout methods
  • 0a623e5: Adds vulnerability detection to prevent use of --upload-pack and --receive-pack without explicitly opting in.

Patch Changes

  • ec97a39: Include restricting the use of git push --exec with other allowUnsafePack exclusions, thanks to @​stsewd for the suggestion.

simple-git@3.15.1

Patch Changes

  • de570ac: Resolves an issue whereby non-strings can be passed into the config switch detector.
Changelog

Sourced from simple-git's changelog.

3.16.0

Minor Changes

  • 97fde2c: Support the use of -B in place of the default -b in checkout methods
  • 0a623e5: Adds vulnerability detection to prevent use of --upload-pack and --receive-pack without explicitly opting in.

Patch Changes

  • ec97a39: Include restricting the use of git push --exec with other allowUnsafePack exclusions, thanks to @​stsewd for the suggestion.

3.15.1

Patch Changes

  • de570ac: Resolves an issue whereby non-strings can be passed into the config switch detector.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=simple-git&package-manager=npm_and_yarn&previous-version=3.15.0&new-version=3.16.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/grafana/oncall/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Joey Orlando --- grafana-plugin/yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/grafana-plugin/yarn.lock b/grafana-plugin/yarn.lock index d3cfc6d8..f06894fb 100644 --- a/grafana-plugin/yarn.lock +++ b/grafana-plugin/yarn.lock @@ -12411,9 +12411,9 @@ signal-exit@^3.0.2, signal-exit@^3.0.3: integrity sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ== simple-git@^3.6.0: - version "3.15.0" - resolved "https://registry.yarnpkg.com/simple-git/-/simple-git-3.15.0.tgz#301a95a943c4f9b0a21d051eb6e6d0ffe4c9754f" - integrity sha512-FiWoMPlcYHQ+ApRihUsGjC/ZmIlWj62S6MBCwOunczvXcLQt+9ZdrysDrR6QVepkRQfEAaBXrN2QtJKrN6zbtg== + version "3.16.0" + resolved "https://registry.yarnpkg.com/simple-git/-/simple-git-3.16.0.tgz#421773e24680f5716999cc4a1d60127b4b6a9dec" + integrity sha512-zuWYsOLEhbJRWVxpjdiXl6eyAyGo/KzVW+KFhhw9MqEEJttcq+32jTWSGyxTdf9e/YCohxRE+9xpWFj9FdiJNw== dependencies: "@kwsites/file-exists" "^1.1.1" "@kwsites/promise-deferred" "^1.1.1" From 4aa1feae06a117d6b8f6610208061a9cd32e1174 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 30 Jan 2023 11:15:17 +0000 Subject: [PATCH 10/16] Add Android app link on mobile app connection tab (#1238) Add the Android app URL to the mobile app connection tab --------- Co-authored-by: Joey Orlando --- .../MobileAppConnection.test.tsx.snap | 217 +++++++++++------- .../__snapshots__/DownloadIcons.test.tsx.snap | 31 ++- .../parts/DownloadIcons/index.tsx | 19 +- 3 files changed, 165 insertions(+), 102 deletions(-) diff --git a/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap b/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap index 6284efb3..56242d64 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap @@ -58,20 +58,27 @@ exports[`MobileAppConnection if we disconnect the app, it disconnects and fetche
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2389,20 +2396,27 @@ exports[`MobileAppConnection it shows a QR code if the app isn't already connect
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2488,20 +2502,27 @@ exports[`MobileAppConnection it shows a loading message if it is currently disco
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2587,20 +2608,27 @@ exports[`MobileAppConnection it shows a loading message if it is currently fetch
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2686,20 +2714,27 @@ exports[`MobileAppConnection it shows a message when the mobile app is already c
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2885,20 +2920,27 @@ exports[`MobileAppConnection it shows an error message if there was an error dis
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2975,20 +3017,27 @@ exports[`MobileAppConnection it shows an error message if there was an error fet
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ diff --git a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap index c355da1b..fe4b7560 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap @@ -52,20 +52,27 @@ exports[`DownloadIcons it renders properly 1`] = `
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ diff --git a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx index 2b140799..d3b68fc8 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx +++ b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx @@ -25,12 +25,19 @@ const DownloadIcons: FC = () => ( iOS - - Play Store - - Android - - + + + Play Store + + Android + + + ); From 8609f415b3c81e613e24eaf910eae9dadc041a73 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 30 Jan 2023 12:36:21 +0100 Subject: [PATCH 11/16] don't tag oncall-backend when CHANGELOG is updated (#1250) # What this PR does There is no need to add `@grafana/grafana-oncall-backend` as a PR reviewer when `CHANGELOG.md` is updated ## Which issue(s) this PR fixes ## Checklist - [ ] Tests updated (N/A) - [ ] Documentation added (N/A) - [ ] `CHANGELOG.md` updated (N/A) --- .github/CODEOWNERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 9484cbb5..b5158c45 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,3 +1,6 @@ * @grafana/grafana-oncall-backend +# don't tag @grafana/grafana-oncall-backend on changes to CHANGELOG.md +CHANGELOG.md + /grafana-plugin @grafana/grafana-oncall-frontend From f80271a1f4ef59bad995024fa2366d84779f8d3f Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 30 Jan 2023 11:48:25 +0000 Subject: [PATCH 12/16] Return alert group ID in direct paging API (#1241) # What this PR does Make direct paging internal API endpoint return an alert group ID. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall/issues/823 ## Checklist - [x] Tests updated --- engine/apps/alerts/paging.py | 6 ++++-- engine/apps/alerts/tests/test_paging.py | 13 +++++++++++++ engine/apps/api/tests/test_paging.py | 2 ++ engine/apps/api/views/paging.py | 4 ++-- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index 12cb0b71..907f1e86 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -1,4 +1,4 @@ -from typing import Any +from typing import Any, Optional from uuid import uuid4 from django.db import transaction @@ -142,7 +142,7 @@ def direct_paging( schedules: ScheduleNotifications = None, escalation_chain: EscalationChain = None, alert_group: AlertGroup = None, -) -> None: +) -> Optional[AlertGroup]: """Trigger escalation targeting given users/schedules. If an alert group is given, update escalation to include the specified users. @@ -185,6 +185,8 @@ def direct_paging( ) notify_user_task.apply_async((u.pk, alert_group.pk), {"important": important}) + return alert_group + def unpage_user(alert_group: AlertGroup, user: User, from_user: User) -> None: """Remove user from alert group escalation.""" diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index e73ccc88..2655d55e 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -279,6 +279,19 @@ def test_direct_paging_custom_chain( assert ag.escalation_chain_with_respect_to_escalation_snapshot == custom_chain +@pytest.mark.django_db +def test_direct_paging_returns_alert_group(make_organization, make_user_for_organization): + organization = make_organization() + user = make_user_for_organization(organization) + from_user = make_user_for_organization(organization) + + with patch("apps.alerts.paging.notify_user_task"): + alert_group = direct_paging(organization, None, from_user, title="Help!", message="Fire", users=[(user, False)]) + + # check alert group returned by direct paging is the same as the one created + assert alert_group == AlertGroup.all_objects.get() + + @pytest.mark.django_db def test_unpage_user_not_exists( make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group diff --git a/engine/apps/api/tests/test_paging.py b/engine/apps/api/tests/test_paging.py index 33b66c09..f2c52e03 100644 --- a/engine/apps/api/tests/test_paging.py +++ b/engine/apps/api/tests/test_paging.py @@ -58,6 +58,7 @@ def test_direct_paging_new_alert_group( ) assert response.status_code == status.HTTP_200_OK + assert "alert_group_id" in response.json() @pytest.mark.django_db @@ -104,6 +105,7 @@ def test_direct_paging_existing_alert_group( ) assert response.status_code == status.HTTP_200_OK + assert response.json()["alert_group_id"] == alert_group.public_primary_key @pytest.mark.django_db diff --git a/engine/apps/api/views/paging.py b/engine/apps/api/views/paging.py index b0fcfd1d..0d44dc95 100644 --- a/engine/apps/api/views/paging.py +++ b/engine/apps/api/views/paging.py @@ -30,7 +30,7 @@ class DirectPagingAPIView(APIView): (schedule["instance"], schedule["important"]) for schedule in serializer.validated_data["schedules"] ] - direct_paging( + alert_group = direct_paging( organization=organization, team=team, from_user=from_user, @@ -42,4 +42,4 @@ class DirectPagingAPIView(APIView): alert_group=serializer.validated_data["alert_group"], ) - return Response(status=status.HTTP_200_OK) + return Response(data={"alert_group_id": alert_group.public_primary_key}, status=status.HTTP_200_OK) From 2b2a837991cf186037bb82acbd1d9e438e0fce7f Mon Sep 17 00:00:00 2001 From: Alyssa Wada <101596687+alyssawada@users.noreply.github.com> Date: Mon, 30 Jan 2023 04:56:30 -0700 Subject: [PATCH 13/16] Add oncall slack commands to docs (#994) # What this PR does Adds instruction for Slack commands and message shortcuts to OnCall docs. ## Which issue(s) this PR fixes Issue #190 --------- Co-authored-by: Joey Orlando Co-authored-by: Joey Orlando Co-authored-by: Matvey Kukuy --- docs/.markdownlint.json | 2 +- .../configure-slack/index.md | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/docs/.markdownlint.json b/docs/.markdownlint.json index 864e4431..a51e7c7e 100644 --- a/docs/.markdownlint.json +++ b/docs/.markdownlint.json @@ -1,7 +1,7 @@ { "extends": "../.markdownlint.json", "MD013": { - "line_length": "140" + "line_length": "160" }, "MD025": false, "MD036": false diff --git a/docs/sources/integrations/chatops-integrations/configure-slack/index.md b/docs/sources/integrations/chatops-integrations/configure-slack/index.md index 75aff6d9..e9595ba6 100644 --- a/docs/sources/integrations/chatops-integrations/configure-slack/index.md +++ b/docs/sources/integrations/chatops-integrations/configure-slack/index.md @@ -81,3 +81,26 @@ teams of their on-call shifts. Admins can configure shift notification behavior 1. When an on-call shift notification is sent to a person or channel, click the gear icon to access **Notifications preferences**. 2. Configure on-call notifications for future shift notifications. + +## Slack commands and message shortcuts + +The Grafana OnCall Slack app includes helpful message shortcuts and slash commands. + +### Slack commands + +Use the `/oncall` Slack command to create a new alert group directly from Slack. + +1. Type `/oncall` in the message box of the desired Slack channel then click **Send**. +1. Fill out the **Start New Escalation** creation form then click **Submit**. +1. Once the Grafana OnCall app sends a Slack message with the newly created alert, the alert group is open and firing. + +### Message shortcuts + +Use message shortcuts to add resolution notes directly from Slack. Message shortcuts are available in the More actions menu from any message. + +>**Note:** In order to associate the resolution note to an alert group, this message shortcut can only be applied to messages in the thread of an alert group. + +1. From an alert group thread, navigate to the Slack message that you wish to add as a resolution note. +1. Hover over the message and select **More actions** from the menu options. +1. Select **Add as resolution note**. +1. The Grafana OnCall app will react to the message in Slack with the memo emoji and add the message to the alert group timeline. From b1fc123d9ffecddef6fd3d2c84385768c2ba4c65 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 30 Jan 2023 09:08:18 -0300 Subject: [PATCH 14/16] Add a filter by involved users to alert groups page (#1240) Related to #1119 It also adds a shortcut to filter current user's related alert groups (alert groups user was notified by, or in which user participated). Make the filter visible by default, with a false value. --- CHANGELOG.md | 4 +++ engine/apps/api/views/alert_group.py | 34 ++++++++++++++++++- .../IncidentsFilters/IncidentsFilters.tsx | 1 + 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f5fde1..06c25579 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 + +- Add involved users filter to alert groups listing page (+ mine shortcut) + ### Changed - Improve logging for creating contact point for Grafana Alerting integration diff --git a/engine/apps/api/views/alert_group.py b/engine/apps/api/views/alert_group.py index d2457a4f..f94ed479 100644 --- a/engine/apps/api/views/alert_group.py +++ b/engine/apps/api/views/alert_group.py @@ -85,7 +85,11 @@ class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.Filt invitees_are = filters.ModelMultipleChoiceFilter( queryset=get_user_queryset, to_field_name="public_primary_key", method="filter_invitees_are" ) + involved_users_are = filters.ModelMultipleChoiceFilter( + queryset=get_user_queryset, to_field_name="public_primary_key", method="filter_by_involved_users" + ) with_resolution_note = filters.BooleanFilter(method="filter_with_resolution_note") + mine = filters.BooleanFilter(method="filter_mine") class Meta: model = AlertGroup @@ -132,10 +136,27 @@ class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.Filt if not users: return queryset - queryset = queryset.filter(acknowledged=False, resolved=False, log_records__author__in=users).distinct() + queryset = queryset.filter(log_records__author__in=users).distinct() return queryset + def filter_by_involved_users(self, queryset, name, value): + users = value + + if not users: + return queryset + + queryset = queryset.filter( + Q(personal_log_records__author__in=users) | Q(log_records__author__in=users) + ).distinct() + + return queryset + + def filter_mine(self, queryset, name, value): + if value: + return self.filter_by_involved_users(queryset, "users", [self.request.user.pk]) + return queryset + def filter_with_resolution_note(self, queryset, name, value): if value is True: queryset = queryset.filter(Q(resolution_notes__isnull=False, resolution_notes__deleted_at=None)).distinct() @@ -522,6 +543,12 @@ class AlertGroupView( "type": "options", "href": api_root + "users/?filters=true&roles=0&roles=1&roles=2", }, + { + "name": "involved_users_are", + "type": "options", + "href": api_root + "users/?filters=true&roles=0&roles=1&roles=2", + "default": {"display_name": self.request.user.username, "value": self.request.user.public_primary_key}, + }, { "name": "status", "type": "options", @@ -548,6 +575,11 @@ class AlertGroupView( "type": "boolean", "default": "true", }, + { + "name": "mine", + "type": "boolean", + "default": "true", + }, ] if filter_name is not None: diff --git a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx index adb54919..55d1e7a1 100644 --- a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx +++ b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx @@ -71,6 +71,7 @@ class IncidentsFilters extends Component Date: Mon, 30 Jan 2023 15:57:20 +0300 Subject: [PATCH 15/16] Add event stop propagation for react router links (#1208) # What this PR does Add stopPropagation for PluginLinks, fixes plugin crash if no start and end present in some working days ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/1253 ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --- .../src/components/PluginLink/PluginLink.tsx | 26 +++++++++++++------ .../WorkingHours/WorkingHours.helpers.ts | 16 +++++++----- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/grafana-plugin/src/components/PluginLink/PluginLink.tsx b/grafana-plugin/src/components/PluginLink/PluginLink.tsx index 2d380a43..4770ec7c 100644 --- a/grafana-plugin/src/components/PluginLink/PluginLink.tsx +++ b/grafana-plugin/src/components/PluginLink/PluginLink.tsx @@ -1,9 +1,8 @@ -import React, { FC, useMemo } from 'react'; +import React, { FC, useCallback, useMemo } from 'react'; import cn from 'classnames/bind'; import { Link } from 'react-router-dom'; -import Text from 'components/Text/Text'; import { getPathFromQueryParams } from 'utils/url'; import styles from './PluginLink.module.css'; @@ -23,12 +22,23 @@ const PluginLink: FC = (props) => { const newPath = useMemo(() => getPathFromQueryParams(query), [query]); - return disabled ? ( - - {children} - - ) : ( - + const handleClick = useCallback( + (event) => { + event.stopPropagation(); + + if (disabled) { + event.preventDefault(); + } + }, + [disabled] + ); + + return ( + {children} ); diff --git a/grafana-plugin/src/components/WorkingHours/WorkingHours.helpers.ts b/grafana-plugin/src/components/WorkingHours/WorkingHours.helpers.ts index 1c3b8f76..4e3226cd 100644 --- a/grafana-plugin/src/components/WorkingHours/WorkingHours.helpers.ts +++ b/grafana-plugin/src/components/WorkingHours/WorkingHours.helpers.ts @@ -86,12 +86,16 @@ export const getNonWorkingMoments = (startMoment, endMoment, workingHours) => { export const isInWorkingHours = (currentMoment: dayjs.Dayjs, workingHours, timezone) => { const timeFormat = 'HH:mm:ss'; const currentDayOfTheWeek = currentMoment.format('dddd').toLowerCase(); - const workingHourStart = workingHours[currentDayOfTheWeek][0].start; - const workingHourEnd = workingHours[currentDayOfTheWeek][0].end; + const workingHourStart = workingHours[currentDayOfTheWeek][0]?.start; + const workingHourEnd = workingHours[currentDayOfTheWeek][0]?.end; - const startTime = dayjs(workingHourStart, timeFormat).tz(timezone).format(timeFormat); - const endTime = dayjs(workingHourEnd, timeFormat).tz(timezone).format(timeFormat); - const currentTime = dayjs(currentMoment, timeFormat).format(timeFormat); + if (workingHourStart && workingHourEnd) { + const startTime = dayjs(workingHourStart, timeFormat).tz(timezone).format(timeFormat); + const endTime = dayjs(workingHourEnd, timeFormat).tz(timezone).format(timeFormat); + const currentTime = dayjs(currentMoment, timeFormat).format(timeFormat); - return currentTime < endTime && currentTime >= startTime; + return currentTime < endTime && currentTime >= startTime; + } + + return false; }; From 7420e11b4f3711d28a18921bd670f3e166373527 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 30 Jan 2023 14:11:42 +0100 Subject: [PATCH 16/16] update CHANGELOG for new release --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06c25579..38ac1685 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## v1.1.20 (2023-01-30) ### Added