diff --git a/.github/workflows/linting-and-tests.yml b/.github/workflows/linting-and-tests.yml index d7b0dcc8..351f698b 100644 --- a/.github/workflows/linting-and-tests.yml +++ b/.github/workflows/linting-and-tests.yml @@ -373,16 +373,14 @@ jobs: uses: actions/cache@v3 with: path: "~/.cache/ms-playwright" - key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }} + key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }}-chromium-firefox-webkit - name: Install Playwright binaries/dependencies if: steps.playwright-cache.outputs.cache-hit != 'true' - # if more browsers are added, will need to modify the "npx playwright install" command # https://stackoverflow.com/questions/65900299/install-single-dependency-from-package-json-with-yarn run: | yarn add "@playwright/test@${{ env.PLAYWRIGHT_VERSION }}" - npx playwright install --with-deps chromium firefox - npx playwright install-deps + npx playwright install --with-deps chromium firefox webkit - name: Await k8s pods and other resources up uses: jupyterhub/action-k8s-await-workloads@v1 diff --git a/CHANGELOG.md b/CHANGELOG.md index cee21a54..278c8470 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ 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). +## v1.2.7 (2023-04-03) + +### Added + +- Save selected teams filter in local storage ([1611](https://github.com/grafana/oncall/issues/1611)) + +### Changed + +- Renamed routes from /incidents to /alert-groups ([#1678](https://github.com/grafana/oncall/pull/1678)) + +### Fixed + +- Fix team search when filtering resources by @vadimkerr ([#1680](https://github.com/grafana/oncall/pull/1680)) +- Fix issue when trying to scroll in Safari ([#415](https://github.com/grafana/oncall/issues/415)) + ## v1.2.6 (2023-03-30) ### Fixed diff --git a/engine/apps/api/tests/test_team.py b/engine/apps/api/tests/test_team.py index 1eb8be04..2f2259cb 100644 --- a/engine/apps/api/tests/test_team.py +++ b/engine/apps/api/tests/test_team.py @@ -45,6 +45,49 @@ def test_list_teams( assert response.json() == expected_payload +@pytest.mark.django_db +@pytest.mark.parametrize( + "search,team_names", + [ + ("", [GENERAL_TEAM.name, "team 1", "team 2"]), + ("team", [GENERAL_TEAM.name, "team 1", "team 2"]), + ("no team", [GENERAL_TEAM.name]), + ("team ", [GENERAL_TEAM.name, "team 1", "team 2"]), + ("team 1", [GENERAL_TEAM.name, "team 1"]), + ], +) +def test_list_teams_search_by_name( + make_organization, + make_team, + make_user_for_organization, + make_token_for_organization, + make_user_auth_headers, + search, + team_names, +): + organization = make_organization() + user = make_user_for_organization(organization) + _, token = make_token_for_organization(organization) + + for team_name in team_names: + if team_name != GENERAL_TEAM.name: + make_team(organization, name=team_name) + + client = APIClient() + + url = reverse("api-internal:team-list") + f"?search={search}" + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + + expected_json = [ + get_payload_from_team(organization.teams.get(name=team_name)) + if team_name != GENERAL_TEAM.name + else get_payload_from_team(GENERAL_TEAM) + for team_name in team_names + ] + assert response.json() == expected_json + + @pytest.mark.django_db def test_list_teams_for_non_member( make_organization, diff --git a/engine/apps/api/views/team.py b/engine/apps/api/views/team.py index a7395483..3cfb827a 100644 --- a/engine/apps/api/views/team.py +++ b/engine/apps/api/views/team.py @@ -1,6 +1,6 @@ from rest_framework import mixins, viewsets +from rest_framework.filters import SearchFilter from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response from apps.api.permissions import RBACPermission from apps.api.serializers.team import TeamSerializer @@ -23,17 +23,15 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod } serializer_class = TeamSerializer + filter_backends = [SearchFilter] + search_fields = ["name"] def get_queryset(self): return self.request.user.available_teams - def list(self, request, *args, **kwargs): - queryset = self.filter_queryset(self.get_queryset()) + def filter_queryset(self, queryset): + """ + Adds general team to the queryset in a way that it always shows up first (even when not searched for). + """ general_team = Team(public_primary_key="null", name="No team", email=None, avatar_url=None) - - page = self.paginate_queryset(queryset) - if page is not None: - serializer = self.get_serializer([general_team] + list(page), many=True) - return self.get_paginated_response(serializer.data) - serializer = self.get_serializer([general_team] + list(queryset), many=True) - return Response(serializer.data) + return [general_team] + list(super().filter_queryset(queryset)) diff --git a/grafana-plugin/integration-tests/alerts/onCallSchedule.test.ts b/grafana-plugin/integration-tests/alerts/onCallSchedule.test.ts index 800faea6..3ba425a9 100644 --- a/grafana-plugin/integration-tests/alerts/onCallSchedule.test.ts +++ b/grafana-plugin/integration-tests/alerts/onCallSchedule.test.ts @@ -6,6 +6,9 @@ import { createIntegrationAndSendDemoAlert } from '../utils/integrations'; import { createOnCallSchedule } from '../utils/schedule'; test('we can create an oncall schedule + receive an alert', async ({ page }) => { + // this test does a lot of stuff, lets give it adequate time to do its thing + test.slow(); + const escalationChainName = generateRandomValue(); const integrationName = generateRandomValue(); const onCallScheduleName = generateRandomValue(); diff --git a/grafana-plugin/integration-tests/utils/forms.ts b/grafana-plugin/integration-tests/utils/forms.ts index 46142e64..10709207 100644 --- a/grafana-plugin/integration-tests/utils/forms.ts +++ b/grafana-plugin/integration-tests/utils/forms.ts @@ -39,7 +39,7 @@ export const clickButton = async ({ dataTestId, }: ClickButtonArgs): Promise => { const baseLocator = dataTestId ? `button[data-testid="${dataTestId}"]` : 'button'; - const button = (startingLocator || page).locator(`${baseLocator} >> text=${buttonText}`); + const button = (startingLocator || page).locator(`${baseLocator}:not([disabled]) >> text=${buttonText}`); await button.waitFor({ state: 'visible' }); await button.click(); diff --git a/grafana-plugin/playwright.config.ts b/grafana-plugin/playwright.config.ts index e1bfa6d8..9084e46a 100644 --- a/grafana-plugin/playwright.config.ts +++ b/grafana-plugin/playwright.config.ts @@ -14,7 +14,6 @@ const config: PlaywrightTestConfig = { testDir: './integration-tests', globalSetup: './integration-tests/globalSetup.ts', /* Maximum time one test can run for. */ - // TODO: set this back to 60 when GSelect component is refactored timeout: 90 * 1000, expect: { /** @@ -28,10 +27,7 @@ const config: PlaywrightTestConfig = { /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, /* Retry on CI only */ - retries: process.env.CI ? 1 : 0, - // TODO: when GSelect component is refactored, run using 3 workers - // locally use one worker, on CI use 3 - // workers: process.env.CI ? 3 : 1, + retries: process.env.CI ? 3 : 0, workers: 1, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: 'html', @@ -64,14 +60,12 @@ const config: PlaywrightTestConfig = { ...devices['Desktop Firefox'], }, }, - - // TODO: enable tests on Safari once the scroll bug when creating an integration is patched - // { - // name: 'webkit', - // use: { - // ...devices['Desktop Safari'], - // }, - // }, + { + name: 'webkit', + use: { + ...devices['Desktop Safari'], + }, + }, /* Test against mobile viewports. */ // { diff --git a/grafana-plugin/src/components/Tutorial/Tutorial.tsx b/grafana-plugin/src/components/Tutorial/Tutorial.tsx index 51a05bee..213e33bd 100644 --- a/grafana-plugin/src/components/Tutorial/Tutorial.tsx +++ b/grafana-plugin/src/components/Tutorial/Tutorial.tsx @@ -66,7 +66,7 @@ const Tutorial: FC = (props) => {
- +
diff --git a/grafana-plugin/src/containers/AlertReceiveChannelCard/AlertReceiveChannelCard.tsx b/grafana-plugin/src/containers/AlertReceiveChannelCard/AlertReceiveChannelCard.tsx index 71c1b2f1..cc4699d5 100644 --- a/grafana-plugin/src/containers/AlertReceiveChannelCard/AlertReceiveChannelCard.tsx +++ b/grafana-plugin/src/containers/AlertReceiveChannelCard/AlertReceiveChannelCard.tsx @@ -65,7 +65,7 @@ const AlertReceiveChannelCard = observer((props: AlertReceiveChannelCardProps) = {alertReceiveChannelCounter && ( (
Demo alert was generated. Find it on the - "Alert Groups" + "Alert Groups" page and make sure it didn't freak out your colleagues 😉
); diff --git a/grafana-plugin/src/containers/AttachIncidentForm/AttachIncidentForm.tsx b/grafana-plugin/src/containers/AttachIncidentForm/AttachIncidentForm.tsx index a7445181..74b110ce 100644 --- a/grafana-plugin/src/containers/AttachIncidentForm/AttachIncidentForm.tsx +++ b/grafana-plugin/src/containers/AttachIncidentForm/AttachIncidentForm.tsx @@ -63,15 +63,15 @@ const AttachIncidentForm = observer(({ id, onUpdate, onHide }: AttachIncidentFor title={ - Attach to another incident + Attach to another alert group } className={cx('root')} onDismiss={onHide} > { {selectedAlertItem ? ( {JSON.stringify(selectedAlertItem, null, 2)} ) : ( - ← Select incident first + ← Select alert group first )}
diff --git a/grafana-plugin/src/containers/IntegrationSettings/IntegrationSettings.tsx b/grafana-plugin/src/containers/IntegrationSettings/IntegrationSettings.tsx index ace4c65d..0154fe2c 100644 --- a/grafana-plugin/src/containers/IntegrationSettings/IntegrationSettings.tsx +++ b/grafana-plugin/src/containers/IntegrationSettings/IntegrationSettings.tsx @@ -59,6 +59,7 @@ const IntegrationSettings = observer((props: IntegrationSettingsProps) => { const [expanded, _setExpanded] = useState(false); const handleSwitchToTemplate = (templateName: string) => { + setActiveTab(IntegrationSettingsTab.Templates); setSelectedTemplate(templateName); }; diff --git a/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.module.css b/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.module.css index c3710fe4..0614492e 100644 --- a/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.module.css +++ b/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.module.css @@ -34,7 +34,12 @@ padding: 4px 8px; margin-top: 8px; min-width: 500px; - width: 520px; + width: 620px; +} + +.autoresolve-div { + display: flex; + align-items: baseline; } .warning-icon-color { diff --git a/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.tsx b/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.tsx index 8d61fcb4..6b4e8c83 100644 --- a/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.tsx +++ b/grafana-plugin/src/containers/IntegrationSettings/parts/Autoresolve.tsx @@ -150,7 +150,7 @@ const Autoresolve = ({ alertReceiveChannelId, onSwitchToTemplate, alertGroupId }
@@ -172,9 +172,9 @@ const Autoresolve = ({ alertReceiveChannelId, onSwitchToTemplate, alertGroupId } {autoresolveSelected && ( <> -
+
- Incident will be automatically resolved when it matches{' '} + Alert group will be automatically resolved when it matches{' '} @@ -244,7 +244,7 @@ class IncidentPage extends React.Component - + {/* @ts-ignore*/} @@ -256,12 +256,12 @@ class IncidentPage extends React.Component {incident.root_alert_group && ( Attached to{' '} - + #{incident.root_alert_group.inside_organization_number}{' '} {incident.root_alert_group.render_for_web.title} {' '} - @@ -421,9 +421,7 @@ class IncidentPage extends React.Component getUnattachClickHandler = (pk: Alert['pk']) => { const { store } = this.props; - return () => { - store.alertGroupStore.unattachAlert(pk).then(this.update); - }; + return store.alertGroupStore.unattachAlert(pk).then(this.update); }; renderTimeline = () => { @@ -762,7 +760,7 @@ function AttachedIncidentsList({ {alerts.map((incident) => { return ( - + #{incident.inside_organization_number} {incident.render_for_web.title} diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index cc4769cc..5c2fe3de 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -127,7 +127,7 @@ class Incidents extends React.Component this.setState({ showAddAlertGroupForm: false }); }} onCreate={(id: Alert['pk']) => { - history.push(`${PLUGIN_ROOT}/incidents/${id}`); + history.push(`${PLUGIN_ROOT}/alert-groups/${id}`); }} /> )} @@ -557,7 +557,13 @@ class Incidents extends React.Component
{record.render_for_web.title} diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index acdeceb4..c3c228be 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -27,24 +27,11 @@ function getPath(name = '') { export const pages: { [id: string]: PageDefinition } = [ { icon: 'bell', - id: 'incidents', + id: 'alert-groups', hideFromBreadcrumbs: true, text: 'Alert Groups', hideTitle: true, - path: getPath('incidents'), - action: UserActions.AlertGroupsRead, - }, - { - icon: 'bell', - id: 'incident', - text: '', - hideFromTabs: true, - hideFromBreadcrumbs: true, - parentItem: { - text: 'Incident', - url: `${PLUGIN_ROOT}/incidents`, - }, - path: getPath('incident'), + path: getPath('alert-groups'), action: UserActions.AlertGroupsRead, }, { @@ -189,8 +176,8 @@ export const pages: { [id: string]: PageDefinition } = [ }, {}); export const ROUTES = { - incidents: ['incidents'], - incident: ['incidents/:id'], + 'alert-groups': ['alert-groups'], + 'alert-group': ['alert-groups/:id'], users: ['users', 'users/:id'], integrations: ['integrations', 'integrations/:id'], escalations: ['escalations', 'escalations/:id'], @@ -205,6 +192,10 @@ export const ROUTES = { 'live-settings': ['live-settings'], cloud: ['cloud'], test: ['test'], + + // backwards compatible to redirect to new alert-groups + incident: ['incidents/:id'], + incidents: ['incidents'], }; export const getRoutesForPage = (name: string) => { diff --git a/grafana-plugin/src/plugin.json b/grafana-plugin/src/plugin.json index 7f10dc71..2ab9172e 100644 --- a/grafana-plugin/src/plugin.json +++ b/grafana-plugin/src/plugin.json @@ -41,7 +41,7 @@ { "type": "page", "name": "Alert Groups", - "path": "/a/grafana-oncall-app/incidents", + "path": "/a/grafana-oncall-app/alert-groups", "role": "Viewer", "action": "grafana-oncall-app.alert-groups:read", "addToNav": true diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index 04fafaf8..4412037f 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -14,7 +14,7 @@ import weekday from 'dayjs/plugin/weekday'; import { observer, Provider } from 'mobx-react'; import Header from 'navbar/Header/Header'; import LegacyNavTabsBar from 'navbar/LegacyNavTabsBar'; -import { Route, Switch, useLocation } from 'react-router-dom'; +import { Redirect, Route, Switch, useLocation } from 'react-router-dom'; import { AppRootProps } from 'types'; import Unauthorized from 'components/Unauthorized'; @@ -138,10 +138,10 @@ export const Root = observer((props: AppRootProps) => { > {userHasAccess ? ( - + - + @@ -183,6 +183,33 @@ export const Root = observer((props: AppRootProps) => { + + {/* Backwards compatibility redirect routes */} + ( + + )} + > + ( + + )} + > + diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index 57e004ba..11c01ed8 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -11,7 +11,7 @@ export const GRAFANA_LICENSE_OSS = 'OpenSource'; export const BREAKPOINT_TABS = 1024; // Default redirect page -export const DEFAULT_PAGE = 'incidents'; +export const DEFAULT_PAGE = 'alert-groups'; export const PLUGIN_ROOT = '/a/grafana-oncall-app'; diff --git a/tools/pagerduty-migrator/README.md b/tools/pagerduty-migrator/README.md index 90e571ef..0b102d9c 100644 --- a/tools/pagerduty-migrator/README.md +++ b/tools/pagerduty-migrator/README.md @@ -86,7 +86,7 @@ pd-oncall-migrator It's possible to specify a default contact method type for user notification rules that cannot be migrated as-is by changing the `ONCALL_DEFAULT_CONTACT_METHOD` env variable. -Options are: `email`, `sms`, `phone_call`, `slack`, `telegram` (default is `email`). +Options are: `email`, `sms`, `phone_call`, `slack`, `telegram`, `mobile_app` (default is `email`). ### After migration diff --git a/tools/pagerduty-migrator/migrator/__main__.py b/tools/pagerduty-migrator/migrator/__main__.py index 403f9232..086273f0 100644 --- a/tools/pagerduty-migrator/migrator/__main__.py +++ b/tools/pagerduty-migrator/migrator/__main__.py @@ -126,7 +126,13 @@ def main() -> None: if rulesets is not None: for ruleset in rulesets: - match_ruleset(ruleset, oncall_integrations, escalation_policies, services) + match_ruleset( + ruleset, + oncall_integrations, + escalation_policies, + services, + integrations, + ) if MODE == MODE_PLAN: print(user_report(users), end="\n\n") diff --git a/tools/pagerduty-migrator/migrator/config.py b/tools/pagerduty-migrator/migrator/config.py index f2baeccb..21c2013d 100644 --- a/tools/pagerduty-migrator/migrator/config.py +++ b/tools/pagerduty-migrator/migrator/config.py @@ -21,7 +21,7 @@ PAGERDUTY_TO_ONCALL_CONTACT_METHOD_MAP = { "sms_contact_method": "notify_by_sms", "phone_contact_method": "notify_by_phone_call", "email_contact_method": "notify_by_email", - "push_notification_contact_method": ONCALL_DEFAULT_CONTACT_METHOD, + "push_notification_contact_method": "notify_by_mobile_app", } PAGERDUTY_TO_ONCALL_VENDOR_MAP = { "Datadog": "datadog", @@ -46,3 +46,7 @@ SCHEDULE_MIGRATION_MODE = os.getenv( EXPERIMENTAL_MIGRATE_EVENT_RULES = ( os.getenv("EXPERIMENTAL_MIGRATE_EVENT_RULES", "false").lower() == "true" ) +# Set to true to include service & integration names in the ruleset name +EXPERIMENTAL_MIGRATE_EVENT_RULES_LONG_NAMES = ( + os.getenv("EXPERIMENTAL_MIGRATE_EVENT_RULES_LONG_NAMES", "false").lower() == "true" +) diff --git a/tools/pagerduty-migrator/migrator/report.py b/tools/pagerduty-migrator/migrator/report.py index b9b6aeef..d63dbd0e 100644 --- a/tools/pagerduty-migrator/migrator/report.py +++ b/tools/pagerduty-migrator/migrator/report.py @@ -189,10 +189,8 @@ def ruleset_report(rulesets: list[dict]) -> str: ): result += "\n" + TAB + format_ruleset(ruleset) if not ruleset["flawed_escalation_policies"] and ruleset["oncall_integration"]: - result += ( - " (existing integration with name '{} Ruleset' will be deleted)".format( - ruleset["name"] - ) + result += " (existing integration with name '{}' will be deleted)".format( + ruleset["oncall_name"] ) return result diff --git a/tools/pagerduty-migrator/migrator/resources/rulesets.py b/tools/pagerduty-migrator/migrator/resources/rulesets.py index a486a0ad..4dd25c64 100644 --- a/tools/pagerduty-migrator/migrator/resources/rulesets.py +++ b/tools/pagerduty-migrator/migrator/resources/rulesets.py @@ -1,4 +1,5 @@ from migrator import oncall_api_client +from migrator.config import EXPERIMENTAL_MIGRATE_EVENT_RULES_LONG_NAMES from migrator.utils import find_by_id @@ -7,14 +8,16 @@ def match_ruleset( oncall_integrations: list[dict], escalation_policies: list[dict], services: list[dict], + integrations: list[dict], ) -> None: # Find existing integration with the same name oncall_integration = None - name = "{} Ruleset".format(ruleset["name"]).lower().strip() + name = _generate_ruleset_name(ruleset, services, integrations) for candidate in oncall_integrations: - if candidate["name"].lower().strip() == name: + if candidate["name"].lower().strip() == name.lower().strip(): oncall_integration = candidate ruleset["oncall_integration"] = oncall_integration + ruleset["oncall_name"] = name # Find services that use escalation policies that cannot be migrated service_ids = [ @@ -52,7 +55,7 @@ def migrate_ruleset( # Create new integration with type "webhook" integration_payload = { - "name": "{} Ruleset".format(ruleset["name"]), + "name": ruleset["oncall_name"], "type": "webhook", "team_id": None, } @@ -163,3 +166,37 @@ def _pd_service_id_to_oncall_escalation_chain_id( escalation_chain_id = escalation_policy["oncall_escalation_chain"]["id"] return escalation_chain_id + + +def _generate_ruleset_name(ruleset, services, integrations): + result = "{} Ruleset".format(ruleset["name"]) + if not EXPERIMENTAL_MIGRATE_EVENT_RULES_LONG_NAMES: + return result + + service_ids = [ + r["actions"]["route"]["value"] + for r in sorted(ruleset["rules"], key=lambda r: r["position"]) + if not r["disabled"] and r["actions"]["route"] + ] + + ruleset_services = [find_by_id(services, service_id) for service_id in service_ids] + ruleset_services = [s for s in ruleset_services if s is not None] + if not ruleset_services: + return result + + service_names = [] + for service in ruleset_services: + service_name = service["name"] + service_integrations = [ + integration + for integration in integrations + if integration["service"]["id"] == service["id"] + ] + if service_integrations: + service_name += " ({})".format( + ", ".join([integration["name"] for integration in service_integrations]) + ) + service_names.append(service_name) + + # OnCall limit for integration name is 150 chars + return "{}: {}".format(result, ", ".join(service_names))[:150]