diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67aa69b2..5f816a2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,6 +27,22 @@ jobs: run: | pre-commit run --all-files + test: + runs-on: ubuntu-latest + container: python:3.9 + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 14.17.0 + - name: Unit Testing Frontend + run: | + pip install $(grep "pre-commit" engine/requirements.txt) + npm install -g yarn + cd grafana-plugin/ + yarn --network-timeout 500000 + yarn test + test-technical-documentation: runs-on: ubuntu-latest steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index 01e27157..4aaef9f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change Log +## v1.0.41 (2022-10-24) +- Add personal email notifications +- Bug fixes + ## v1.0.40 (2022-10-05) - Improved database and celery backends support - Added script to import PagerDuty users to Grafana diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 2666ade7..caa79fbf 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -385,10 +385,19 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): organization=kwargs["organization"], integration=kwargs["integration"], team=kwargs["team"], + deleted_at=None, ) except cls.DoesNotExist: kwargs.update(defaults) alert_receive_channel = cls.create(**kwargs) + except cls.MultipleObjectsReturned: + # general team may inherit integrations from deleted teams + alert_receive_channel = cls.objects.filter( + organization=kwargs["organization"], + integration=kwargs["integration"], + team=kwargs["team"], + deleted_at=None, + ).first() return alert_receive_channel @property diff --git a/engine/apps/alerts/tests/test_alert_receiver_channel.py b/engine/apps/alerts/tests/test_alert_receiver_channel.py index 20d5528d..cdc204de 100644 --- a/engine/apps/alerts/tests/test_alert_receiver_channel.py +++ b/engine/apps/alerts/tests/test_alert_receiver_channel.py @@ -144,3 +144,28 @@ def test_notify_maintenance_with_general_channel(make_organization, make_alert_r mock_post_message.assert_called_once_with( organization, organization.general_log_channel_id, "maintenance mode enabled" ) + + +@pytest.mark.django_db +def test_get_or_create_manual_integration_deleted_team(make_organization, make_team, make_alert_receive_channel): + organization = make_organization(general_log_channel_id="CHANNEL-ID") + # setup general manual integration + general_manual = AlertReceiveChannel.get_or_create_manual_integration( + organization=organization, team=None, integration=AlertReceiveChannel.INTEGRATION_MANUAL, defaults={} + ) + # setup another team manual integration + team1 = make_team(organization) + team1_manual = AlertReceiveChannel.get_or_create_manual_integration( + organization=organization, team=team1, integration=AlertReceiveChannel.INTEGRATION_MANUAL, defaults={} + ) + + # team is deleted + team1.delete() + team1_manual.refresh_from_db() + assert team1_manual.team is None + + # it should still be possible to get a manual integration for general team + integration = AlertReceiveChannel.get_or_create_manual_integration( + organization=organization, team=None, integration=AlertReceiveChannel.INTEGRATION_MANUAL, defaults={} + ) + assert integration == general_manual diff --git a/engine/apps/api/serializers/schedule_base.py b/engine/apps/api/serializers/schedule_base.py index b3e1858d..c2eb6ea6 100644 --- a/engine/apps/api/serializers/schedule_base.py +++ b/engine/apps/api/serializers/schedule_base.py @@ -76,7 +76,8 @@ class ScheduleBaseSerializer(EagerLoadingMixin, serializers.ModelSerializer): def get_number_of_escalation_chains(self, obj): # num_escalation_chains param added in queryset via annotate. Check ScheduleView.get_queryset # return 0 for just created schedules - return getattr(obj, "num_escalation_chains", 0) + num = getattr(obj, "num_escalation_chains", 0) + return num or 0 def validate(self, attrs): if "slack_channel_id" in attrs: diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 4bc5764e..9c52a69c 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -13,7 +13,7 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.views import Response from rest_framework.viewsets import ModelViewSet -from apps.alerts.models import EscalationChain +from apps.alerts.models import EscalationChain, EscalationPolicy from apps.api.permissions import MODIFY_ACTIONS, READ_ACTIONS, ActionPermission, AnyRole, IsAdmin, IsAdminOrEditor from apps.api.serializers.schedule_base import ScheduleFastSerializer from apps.api.serializers.schedule_polymorphic import ( @@ -108,22 +108,28 @@ class ScheduleView( slack_team_identity=organization.slack_team_identity, slack_id=OuterRef("channel"), ) + escalation_policies = ( + EscalationPolicy.objects.values("notify_schedule") + .order_by("notify_schedule") + .annotate(num_escalation_chains=Count("notify_schedule")) + .filter(notify_schedule=OuterRef("id")) + ) queryset = queryset.annotate( slack_channel_name=Subquery(slack_channels.values("name")[:1]), slack_channel_pk=Subquery(slack_channels.values("public_primary_key")[:1]), - num_escalation_chains=Count( - "escalation_policies__escalation_chain", - distinct=True, - ), + num_escalation_chains=Subquery(escalation_policies.values("num_escalation_chains")[:1]), ) return queryset def get_queryset(self): is_short_request = self.request.query_params.get("short", "false") == "true" organization = self.request.auth.organization - queryset = OnCallSchedule.objects.filter( - organization=organization, - team=self.request.user.current_team, + queryset = OnCallSchedule.objects.filter(organization=organization, team=self.request.user.current_team).defer( + # avoid requesting large text fields which are not used when listing schedules + "cached_ical_file_primary", + "prev_ical_file_primary", + "cached_ical_file_overrides", + "prev_ical_file_overrides", ) if not is_short_request: queryset = self._annotate_queryset(queryset) diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index ac8f9596..3e6756cd 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -42,13 +42,13 @@ def users_in_ical(usernames_from_ical, organization, include_viewers=False): Parse ical file and return list of users found """ # Only grafana username will be used, consider adding grafana email and id - users_found_in_ical = organization.users if not include_viewers: users_found_in_ical = users_found_in_ical.filter(role__in=(Role.ADMIN, Role.EDITOR)) + user_emails = [v.lower() for v in usernames_from_ical] users_found_in_ical = users_found_in_ical.filter( - (Q(username__in=usernames_from_ical) | Q(email__in=usernames_from_ical)) + (Q(username__in=usernames_from_ical) | Q(email__lower__in=user_emails)) ).distinct() # Here is the example how we extracted users previously, using slack fields too @@ -394,8 +394,8 @@ def get_missing_users_from_ical_event(event, organization): all_usernames, _ = get_usernames_from_ical_event(event) users = list(get_users_from_ical_event(event, organization)) found_usernames = [u.username for u in users] - found_emails = [u.email for u in users] - return [u for u in all_usernames if u != "" and u not in found_usernames and u not in found_emails] + found_emails = [u.email.lower() for u in users] + return [u for u in all_usernames if u != "" and u not in found_usernames and u.lower() not in found_emails] def get_users_from_ical_event(event, organization): @@ -536,7 +536,8 @@ def get_user_events_from_calendars(ical_obj: Calendar, calendars: tuple, user: U for component in calendar.walk(): if component.name == "VEVENT": event_user = get_usernames_from_ical_event(component) - if event_user[0][0] in [user.username, user.email]: + event_user_value = event_user[0][0] + if event_user_value == user.username or event_user_value.lower() == user.email.lower(): ical_obj.add_component(component) diff --git a/engine/apps/schedules/tests/test_ical_utils.py b/engine/apps/schedules/tests/test_ical_utils.py index f6bc2155..b1d7171f 100644 --- a/engine/apps/schedules/tests/test_ical_utils.py +++ b/engine/apps/schedules/tests/test_ical_utils.py @@ -15,6 +15,16 @@ from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar from common.constants.role import Role +@pytest.mark.django_db +def test_users_in_ical_email_case_insensitive(make_organization_and_user, make_user_for_organization): + organization, user = make_organization_and_user() + user = make_user_for_organization(organization, username="foo", email="TestingUser@test.com") + + usernames = ["testinguser@test.com"] + result = users_in_ical(usernames, organization) + assert set(result) == {user} + + @pytest.mark.django_db @pytest.mark.parametrize( "include_viewers", diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index c22a14ad..0d1831be 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -358,7 +358,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): author_verbal = resolution_note.author_verbal(mention=True) resolution_note_text_block = { "type": "section", - "text": {"type": "plain_text", "text": resolution_note.text, "emoji": True}, + "text": {"type": "mrkdwn", "text": resolution_note.text, "emoji": True}, } blocks.append(resolution_note_text_block) context_block = { diff --git a/engine/apps/user_management/apps.py b/engine/apps/user_management/apps.py new file mode 100644 index 00000000..2dd0c259 --- /dev/null +++ b/engine/apps/user_management/apps.py @@ -0,0 +1,16 @@ +from django.apps import AppConfig +from django.db import models + + +# enable a __lower field lookup for email fields +# https://docs.djangoproject.com/en/4.1/howto/custom-lookups/#a-bilateral-transformer-example +class LowerCase(models.Transform): + lookup_name = "lower" + function = "LOWER" + + +class UserManagementConfig(AppConfig): + name = "apps.user_management" + + def ready(self): + models.EmailField.register_lookup(LowerCase) diff --git a/engine/apps/user_management/tests/test_user.py b/engine/apps/user_management/tests/test_user.py index fe615c7c..74440cd1 100644 --- a/engine/apps/user_management/tests/test_user.py +++ b/engine/apps/user_management/tests/test_user.py @@ -2,6 +2,7 @@ import pytest +from apps.user_management.models import User from common.constants.role import Role @@ -22,3 +23,16 @@ def test_self_or_admin( assert admin.self_or_admin(editor, organization) is False assert admin.self_or_admin(second_admin, organization) is True assert admin.self_or_admin(admin_from_another_organization, organization) is False + + +@pytest.mark.django_db +def test_lower_email_filter( + make_organization, + make_user_for_organization, +): + organization = make_organization() + user = make_user_for_organization(organization, email="TestingUser@test.com") + make_user_for_organization(organization, email="testing_user@test.com") + + assert User.objects.get(email__lower="testinguser@test.com") == user + assert User.objects.filter(email__lower__in=["testinguser@test.com"]).get() == user diff --git a/grafana-plugin/jest.config.js b/grafana-plugin/jest.config.js index bcf17c90..31684c09 100644 --- a/grafana-plugin/jest.config.js +++ b/grafana-plugin/jest.config.js @@ -1,8 +1,29 @@ -// This file is needed because it is used by vscode and other tools that -// call `jest` directly. However, unless you are doing anything special -// do not edit this file +module.exports = { + preset: 'ts-jest', + testEnvironment: 'jsdom', -const standard = require('@grafana/toolkit/src/config/jest.plugin.config'); + moduleDirectories: ['node_modules', 'src'], + moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'], -// This process will use the same config that `yarn test` is using -module.exports = standard.jestConfig(); + globals: { + 'ts-jest': { + isolatedModules: true, + babelConfig: true + }, + }, + + transform: { + '^.+\\.js?$': require.resolve('babel-jest'), + '^.+\\.jsx?$': require.resolve('babel-jest'), + '^.+\\.ts?$': require.resolve('ts-jest'), + '^.+\\.tsx?$': require.resolve('ts-jest'), + }, + + moduleNameMapper: { + "grafana/app/(.*)": '/src/jest/grafanaMock.ts', + "jest/outgoingWebhooksStub": '/src/jest/outgoingWebhooksStub.ts', + "^jest$": '/src/jest', + '^.+\\.(css|scss)$': '/src/jest/styleMock.ts', + "^lodash-es$": "lodash", + } +}; diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 0093af7d..021f0224 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -8,7 +8,7 @@ "stylelint": "stylelint ./src/**/*.css", "stylelint:fix": "stylelint --fix ./src/**/*.css", "build": "grafana-toolkit plugin:build", - "test": "grafana-toolkit plugin:test", + "test": "jest --verbose", "dev": "grafana-toolkit plugin:dev", "watch": "grafana-toolkit plugin:dev --watch", "sign": "grafana-toolkit plugin:sign", @@ -55,21 +55,30 @@ "@grafana/runtime": "^9.1.1", "@grafana/toolkit": "^9.1.1", "@grafana/ui": "^9.1.1", + "@jest/globals": "^27.5.1", + "@testing-library/jest-dom": "^5.16.5", + "@testing-library/react": "12", "@types/dompurify": "^2.3.4", + "@types/jest": "^27.5.1", "@types/lodash-es": "^4.17.6", "@types/react-copy-to-clipboard": "^5.0.4", "@types/react-dom": "^18.0.6", "@types/react-responsive": "^8.0.5", "@types/react-router-dom": "^5.3.3", + "@types/react-test-renderer": "^17.0.2", "@types/throttle-debounce": "^5.0.0", "copy-webpack-plugin": "^11.0.0", "dompurify": "^2.3.12", "eslint-plugin-rulesdir": "^0.2.1", + "jest": "^27.5.1", + "jest-environment-jsdom": "^27.5.1", "lint-staged": "^10.2.11", "lodash-es": "^4.17.21", "moment-timezone": "^0.5.35", "plop": "^2.7.4", "postcss-loader": "^7.0.1", + "react-test-renderer": "^17.0.2", + "ts-jest": "^27.1.3", "ts-loader": "^9.3.1", "webpack-bundle-analyzer": "^4.6.1" }, diff --git a/grafana-plugin/src/components/Avatar/Avatar.test.tsx b/grafana-plugin/src/components/Avatar/Avatar.test.tsx new file mode 100644 index 00000000..9c9053da --- /dev/null +++ b/grafana-plugin/src/components/Avatar/Avatar.test.tsx @@ -0,0 +1,26 @@ +import React from 'react'; + +import { describe, expect, test } from '@jest/globals'; +import { render, fireEvent, screen } from '@testing-library/react'; + +import Avatar from 'components/Avatar/Avatar'; + +import '@testing-library/jest-dom'; + +describe('Avatar', () => { + const avatarSrc = 'http://avatar.com/'; + const avatarSizeLarge = 'large'; + const avatarSizeSmall = 'small'; + + test("Avatar's image points to given src attribute", async () => { + render(); + const imageEl = await screen.findByTestId('test__avatar'); + expect(imageEl.src).toBe(avatarSrc); + }); + + test('Avatar appends sizing class', async () => { + render(); + const imageEl = await screen.findByTestId('test__avatar'); + expect(imageEl.classList).toContain(`avatarSize-${avatarSizeSmall}`); + }); +}); diff --git a/grafana-plugin/src/components/Avatar/Avatar.tsx b/grafana-plugin/src/components/Avatar/Avatar.tsx index 15b93552..85c7a3c6 100644 --- a/grafana-plugin/src/components/Avatar/Avatar.tsx +++ b/grafana-plugin/src/components/Avatar/Avatar.tsx @@ -19,7 +19,7 @@ const Avatar: FC = (props) => { return null; } - return ; + return ; }; export default Avatar; diff --git a/grafana-plugin/src/components/CardButton/CardButton.test.tsx b/grafana-plugin/src/components/CardButton/CardButton.test.tsx new file mode 100644 index 00000000..05c29427 --- /dev/null +++ b/grafana-plugin/src/components/CardButton/CardButton.test.tsx @@ -0,0 +1,37 @@ +import 'jest/matchMedia.ts'; +import React from 'react'; + +import { describe, expect, test } from '@jest/globals'; +import { fireEvent, render, screen } from '@testing-library/react'; + + +import '@testing-library/jest-dom'; +import CardButton from 'components/CardButton/CardButton'; + +describe('CardButton', () => { + function getProps(onClickMock: jest.Mock = jest.fn()) { + return { + icon: <>, + description: 'Description', + title: 'Title', + selected: true, + onClick: onClickMock, + }; + } + + test('It updates class and calls onClick prop on click', () => { + const onClickMock = jest.fn(); + render(); + + const rootEl = getRootBlockEl() + + fireEvent.click(rootEl); + + expect(rootEl.classList).toContain("root_selected") + expect(onClickMock).toHaveBeenCalled(); + }); + + function getRootBlockEl(): HTMLElement { + return screen.queryByTestId('test__cardButton'); + } +}); diff --git a/grafana-plugin/src/components/CardButton/CardButton.tsx b/grafana-plugin/src/components/CardButton/CardButton.tsx index 40259e3c..95d3ea6d 100644 --- a/grafana-plugin/src/components/CardButton/CardButton.tsx +++ b/grafana-plugin/src/components/CardButton/CardButton.tsx @@ -26,7 +26,7 @@ const CardButton: FC = (props) => { }, [selected]); return ( - +
{icon}
diff --git a/grafana-plugin/src/components/Collapse/Collapse.test.tsx b/grafana-plugin/src/components/Collapse/Collapse.test.tsx new file mode 100644 index 00000000..2468af8a --- /dev/null +++ b/grafana-plugin/src/components/Collapse/Collapse.test.tsx @@ -0,0 +1,53 @@ +import 'jest/matchMedia.ts'; +import React from 'react'; + +import { describe, expect, test } from '@jest/globals'; +import { render, fireEvent, screen } from '@testing-library/react'; + +import Collapse, { CollapseProps } from 'components/Collapse/Collapse'; + +import '@testing-library/jest-dom'; + +describe('Collapse', () => { + function getProps(isOpen: boolean, onClick: jest.Mock = jest.fn()) { + return { + label: 'Toggle', + isOpen: isOpen, + onClick: onClick + } as CollapseProps + } + + test('Content becomes visible on click', () => { + render(); + + const hiddenChildrenContent = getChildrenEl(); + expect(hiddenChildrenContent).toBeNull(); + + const toggler = getTogglerEl(); + fireEvent.click(toggler); + + expect(hiddenChildrenContent).toBeDefined(); + }); + + test('Content is collapsed for [isOpen=false]', () => { + render(); + + const content = getChildrenEl(); + expect(content).toBeNull(); + }) + + test('Content is not collapsed for [isOpen=true]', () => { + render(); + + const content = getChildrenEl(); + expect(content).toBeDefined(); + }); + + function getChildrenEl(): HTMLElement { + return screen.queryByTestId('test__children'); + } + + function getTogglerEl(): HTMLElement { + return screen.queryByTestId('test__toggle'); + } +}); diff --git a/grafana-plugin/src/components/Collapse/Collapse.tsx b/grafana-plugin/src/components/Collapse/Collapse.tsx index 940507aa..27644e0d 100644 --- a/grafana-plugin/src/components/Collapse/Collapse.tsx +++ b/grafana-plugin/src/components/Collapse/Collapse.tsx @@ -3,11 +3,9 @@ import React, { FC, useCallback, useState } from 'react'; import { Icon } from '@grafana/ui'; import cn from 'classnames/bind'; -import Block from 'components/GBlock/Block'; - import styles from 'components/Collapse/Collapse.module.css'; -interface CollapseProps { +export interface CollapseProps { label: React.ReactNode; isOpen: boolean; onToggle?: (isOpen: boolean) => void; @@ -45,11 +43,19 @@ const Collapse: FC = (props) => { return (
-
+
{label}
- {isOpen &&
{children}
} + {isOpen && ( +
+ {children} +
+ )}
); }; diff --git a/grafana-plugin/src/components/GForm/GForm.tsx b/grafana-plugin/src/components/GForm/GForm.tsx index 367b420d..5ba31cd5 100644 --- a/grafana-plugin/src/components/GForm/GForm.tsx +++ b/grafana-plugin/src/components/GForm/GForm.tsx @@ -99,9 +99,10 @@ const GForm = (props: GFormProps) => { return (
{({ register, errors, control }) => { - return form.fields.map((formItem: FormItem) => { + return form.fields.map((formItem: FormItem, formIndex: number) => { return ( = (props) => { [data] ); - /* useEffect(() => { // todo clear selection on data change - if (rowSelection && rowSelection.selectedRowKeys.length) { - const { selectedRowKeys, onChange } = rowSelection; - const newSelectedRowKeys = selectedRowKeys.filter((key: string) => - data.some((item: any) => item[rowKey as string] === key) - ); - onChange(newSelectedRowKeys); - } - }, [data?.length]); */ - const columns = useMemo(() => { const columns = [...columnsProp]; @@ -146,7 +136,7 @@ const GTable: FC = (props) => { }, [rowSelection, columnsProp, data]); return ( -
+
{ + test("SourceCode doesn't render clipboard for [showCopyToClipboard=false]", () => { + render(); + const codeEl = screen.queryByRole('code'); + expect(codeEl).toBeNull(); + }); + + test('SourceCode renders clipboard for [showCopyToClipboard=true]', () => { + render(); + const codeEl = screen.queryByRole('code'); + expect(codeEl).toBeDefined(); + }); + + test('SourceCode displays just copy icon for [showClipboardIconOnly=true]', () => { + render(); + expect(screen.queryByTestId('test__copyIcon')).toBeDefined(); + expect(screen.queryByTestId('test__copyIconWithText')).toBeNull(); + }); + + test('SourceCode displays copy icon and text for [showClipboardIconOnly=false]', () => { + render(); + expect(screen.queryByTestId('test__copyIcon')).toBeNull(); + expect(screen.queryByTestId('test__copyIconWithText')).toBeDefined(); + }); +}); diff --git a/grafana-plugin/src/components/SourceCode/SourceCode.tsx b/grafana-plugin/src/components/SourceCode/SourceCode.tsx index de97190c..dc1c3f89 100644 --- a/grafana-plugin/src/components/SourceCode/SourceCode.tsx +++ b/grafana-plugin/src/components/SourceCode/SourceCode.tsx @@ -31,9 +31,9 @@ const SourceCode: FC = (props) => { }} > {showClipboardIconOnly ? ( - + ) : ( - )} diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx index 76891c47..47317d42 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx @@ -54,7 +54,7 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { onClose={onHide} closeOnMaskClick > -
+