Skip to content

Commit

Permalink
Merge branch 'dev' into mderynck/update-escalation-step-description
Browse files Browse the repository at this point in the history
  • Loading branch information
mderynck committed May 22, 2024
2 parents 0a48637 + c830eae commit c52d79f
Show file tree
Hide file tree
Showing 110 changed files with 4,136 additions and 1,384 deletions.
8 changes: 3 additions & 5 deletions .drone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ steps:
- refs/tags/v*.*.*

- name: Build and Push Engine Docker Image Backend to GCR
image: plugins/docker
image: plugins/docker:20.17.4
environment:
# force docker to use buildkit feature, this will skip build stages that aren't required in
# the final image (ie. dev & dev-enterprise)
Expand All @@ -116,7 +116,7 @@ steps:
- Image Tag

- name: Build and Push Engine Docker Image Backend to Dockerhub
image: plugins/docker
image: plugins/docker:20.17.4
environment:
# force docker to use buildkit feature, this will skip build stages that aren't required in
# the final image (ie. dev & dev-enterprise)
Expand Down Expand Up @@ -386,6 +386,4 @@ name: cloud_access_policy_token

---
kind: signature
hmac: a045d72f3f3510895da049f4bf8f5ae4ac21f3ffa3d24cda047152c286df5bc2

...
hmac: 7bf9c1d378bf2a93cb758436de78878f9a49a8501b5d1b199c412198439d3593
10 changes: 5 additions & 5 deletions .github/workflows/linting-and-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -279,18 +279,18 @@ jobs:
uv pip sync --system requirements.txt requirements-dev.txt
pytest -x
unit-test-pd-migrator:
name: "Unit tests - PagerDuty Migrator"
unit-test-migrators:
name: "Unit tests - Migrators"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: "3.11.4"
cache: "pip"
cache-dependency-path: tools/pagerduty-migrator/requirements.txt
- name: Unit Test PD Migrator
working-directory: tools/pagerduty-migrator
cache-dependency-path: tools/migrators/requirements.txt
- name: Unit Test Migrators
working-directory: tools/migrators
run: |
pip install uv
uv pip sync --system requirements.txt
Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/on-issue-creation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ jobs:
is ${{ steps.get-latest-tag.outputs.tag }}. If your issue pertains to an older version of Grafana OnCall,
please be sure to list it in the PR description. Thank you :smile:!
add-needs-triage-label:
name: Add "needs triage" label
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Add "needs triage" label
uses: actions-ecosystem/action-add-labels@v1
with:
labels: needs triage

map-selected-product-areas-to-labels-and-assignees:
name: Map selected product areas to labels and assignees
runs-on: ubuntu-latest
Expand Down
15 changes: 7 additions & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ repos:
files: ^engine
args: [--settings-file=engine/pyproject.toml, --filter-files]
- id: isort
name: isort - pd-migrator
files: ^tools/pagerduty-migrator
args:
[--settings-file=tools/pagerduty-migrator/.isort.cfg, --filter-files]
name: isort - migrators
files: ^tools/migrators
args: [--settings-file=tools/migrators/.isort.cfg, --filter-files]
- id: isort
name: isort - dev/scripts
files: ^dev/scripts
Expand All @@ -22,8 +21,8 @@ repos:
files: ^engine
args: [--config=engine/pyproject.toml]
- id: black
name: black - pd-migrator
files: ^tools/pagerduty-migrator
name: black - migrators
files: ^tools/migrators
- id: black
name: black - dev/scripts
files: ^dev/scripts
Expand All @@ -38,8 +37,8 @@ repos:
- flake8-bugbear
- flake8-tidy-imports
- id: flake8
name: flake8 - pd-migrator
files: ^tools/pagerduty-migrator
name: flake8 - migrators
files: ^tools/migrators
# Make sure config is compatible with black
# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8
args: ["--max-line-length=88", "--extend-ignore=E203,E501"]
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ Have a question, comment or feedback? Don't be afraid to [open an issue](https:/

## Further Reading

- _Migration from PagerDuty_ - [Migrator](https://github.com/grafana/oncall/tree/dev/tools/pagerduty-migrator)
- _Automated migration from other on-call tools_ - [Migrator](https://github.com/grafana/oncall/tree/dev/tools/migrators)
- _Documentation_ - [Grafana OnCall](https://grafana.com/docs/oncall/latest/)
- _Overview Webinar_ - [YouTube](https://www.youtube.com/watch?v=7uSe1pulgs8)
- _How To Add Integration_ - [How to Add Integration](https://github.com/grafana/oncall/tree/dev/engine/config_integrations/README.md)
Expand Down
1 change: 1 addition & 0 deletions dev/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
!add_env_var.sh
!prometheus.yml
!README.md
!frontend_guidelines.md
!scripts/
!scripts/*
!helm-local.yml
Expand Down
124 changes: 124 additions & 0 deletions dev/frontend_guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# OnCall Frontend Guidelines

The base guidelines we decided to follow are:

- <https://github.com/ryanmcdermott/clean-code-javascript>
- <https://github.com/grafana/grafana/tree/main/contribute/style-guides>

Anything that is either not covered by the materials above or we decided to do differently is stated in this document.

## Ways of working

### Code reviews

- PR can be merged when the following conditions are met:
- CI pipeline succeeded without bypassing checks
- There is at least 1 approval from the frontend team and no opened remarks from reviewers other than the approver
- All comments are replied, answered, addressed or discussed separately
- We make use of builtin GH PR reviews
- Minor comments that don’t need to be addressed right away and are not a blocker for a merge
are marked with “Minor: “ prefix
- *Unless there is some important hotfix to make and no frontend mates are available*

## Technical conventions

### Type-safety

- Don’t use implicit / explicit `any` or @ts-ignore

### Use auto generated types for new features

- For new features and endpoints that are covered by OpenApi schemas, we should use auto generated types and
typed http client

### Naming conventions on most used variables such as handlers, booleans etc

- Use descriptive naming (such as the use of is) that follows natural convention such as `isFormVerified`
or `isAuthenticated` instead of *verified*, *authenticated* etc
- Use descriptive naming for event handlers such as `onIconClick` or `onInputChange` rather than *handleChange*
- Include the **verb** part in the functions’ names
- For **3 or more** function arguments use object destructuring
<https://github.com/ryanmcdermott/clean-code-javascript?tab=readme-ov-file#function-arguments-2-or-fewer-ideally>
- For functions that return other functions getBlaBlaHandler (in case of handlers) or getBlaBlaFn
(if it’s not handler that is returned)
- Don’t use higher-ordered functions without clear need

## State management

### Don’t use returned values from MobX actions

- MobX actions should not return value that is later used within components. Instead MobX actions should update
observables and then components should consume observables to reflect data updates.

### Leverage small global stores over local state passed as props over multiple levels

- Create small specialized store in MobX even for small feature
instead of relying on local state that is passed down the React tree over multiple levels
e.g.:
alert_receive_channel_connected_channels.ts
alert_receive_channel_webhooks.ts

### Use global decorators / custom hooks / utilities consistently

- For example:
- `@WithGlobalNotification` to set successful / failing notification based on MobX action’s result
- `@AutoLoadingState` to manage loading statuses of async actions
- `useIsLoading` to consume loading state
- `useDrawer` to manage drawers
- `useConfirmModal` to manage confirm modal
- Global helpers

## Code organization

### Store files in appropriate places

- Store files in appropriate places (components, containers, models etc)
- Helpers and configs should be stored in the same folder with the appropriate name
e.g. [container].helper.ts, [container].config.ts
- Project-wide utilities should be placed in the **utils** folder`
- Use named exports for all code you want to export from a file.
- Export only the code that is meant to be used outside the module.

### Don’t opt-out from eslint / TS rules

- Avoid opting out from eslint or TS rules unless there is a strong reason / lack of possibility to follow the rule

## Styling

### Use Emotion.js with agreed code style

- Use emotion.js for styling
- Make use of `useStyles2` hook
- Use css `` syntax
- Place `getStyles` function at the end of the same file or in the separate file with `X.styles.ts` suffix
(if it’s reused by multiple components or it’s too large for placing in the same component’s file)

## React

### Keep components functional and small

- Use functional components for new features
- Leverage components composition, use many small components, render list items in dedicated component
and dereference values late
<https://mobx.js.org/react-optimizations.html>
- Keep components small and flat
- Use dedicated components over render functions in case a there's a risk that a single component grows too much
- Static values (especially non-primitive) that don’t depend on local state, props or store should be extracted
out of component
- Don’t create unnecessary local state that duplicates parts of global store
- Leverage custom hooks to extract repetitive logic
- Don’t use useMemo / useCallback by default

## Architecture

### Use layered architecture

- Don’t interact with backend directly from components
- Don’t interact with 3rd party (faro, web storage etc) directly but rather through service

## Frontend-backend communication

### Use typed HTTP request and response payloads

- HTTP request payload and response payload should always be typed
- Use typed http client whenever we have auto-generated types available
2 changes: 1 addition & 1 deletion dev/scripts/generate-fake-data/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
aiohttp==3.9.4
Faker==16.4.0
tqdm==4.64.1
tqdm==4.66.3
3 changes: 2 additions & 1 deletion docs/sources/configure/jinja2-templating/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,10 @@ Built-in functions:
- `tojson` - dumps a structure to JSON
- `tojson_pretty` - same as tojson, but prettified
- `iso8601_to_time` - converts time from iso8601 (`2015-02-17T18:30:20.000Z`) to datetime
- `datetimeformat` - converts time from datetime to the given format (`%H:%M / %d-%m-%Y` by default)
- `datetimeformat` - converts datetime to string according to strftime format codes (`%H:%M / %d-%m-%Y` by default)
- `datetimeformat_as_timezone` - same as `datetimeformat`, with the inclusion of timezone conversion (`UTC` by default)
- Usage example: `{{ payload.alerts.startsAt | iso8601_to_time | datetimeformat_as_timezone('%Y-%m-%dT%H:%M:%S%z', 'America/Chicago') }}`
- `datetimeparse` - converts string to datetime according to strftime format codes (`%H:%M / %d-%m-%Y` by default)
- `regex_replace` - performs a regex find and replace
- `regex_match` - performs a regex match, returns `True` or `False`
- Usage example: `{{ payload.ruleName | regex_match(".*") }}`
Expand Down
2 changes: 2 additions & 0 deletions docs/sources/manage/insights-and-metrics/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ This metric has the following labels:
| `org_id` | ID of Grafana organization |
| `team` | Team name |
| `integration` | OnCall Integration name |
| `service_name`| Value of Alert group `service_name` label |
| `state` | Alert groups state. May be `firing`, `acknowledged`, `resolved` and `silenced`|

**Query example:**
Expand All @@ -86,6 +87,7 @@ This metric has the following labels:
| `org_id` | ID of Grafana organization |
| `team` | Team name |
| `integration` | OnCall Integration name |
| `service_name`| Value of Alert group `service_name` label |
| `le` | Histogram bucket value in seconds. May be `60`, `300`, `600`, `3600` and `+Inf`|

**Query example:**
Expand Down
10 changes: 7 additions & 3 deletions docs/sources/set-up/migration-from-other-tools/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ keywords:
- OnCall
- Migration
- Pagerduty
- Splunk OnCall
- VictorOps
- on-call tools
canonical: https://grafana.com/docs/oncall/latest/set-up/migration-from-other-tools/
aliases:
Expand All @@ -17,7 +19,9 @@ aliases:

# Migration from other tools

## Migration from PagerDuty to Grafana OnCall
We currently support automated migration from the following on-call tools:

Migration from PagerDuty to Grafana OnCall could be performed in automated way using
[OSS Migrator](https://github.com/grafana/oncall/tree/dev/tools/pagerduty-migrator).
- PagerDuty
- Splunk OnCall (VictorOps)

See our [OSS Migrator](https://github.com/grafana/oncall/tree/dev/tools/migrators) for more details.
2 changes: 1 addition & 1 deletion engine/apps/api/serializers/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def get_payload_example(self, obj: "AlertReceiveChannel"):
raise serializers.ValidationError("Unable to retrieve example payload for this alert group")
else:
try:
return obj.alert_groups.last().alerts.first().raw_request_data
return obj.alert_groups.only("id").last().alerts.first().raw_request_data
except AttributeError:
return None

Expand Down
42 changes: 41 additions & 1 deletion engine/apps/api/tests/test_schedules.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import json
import textwrap
from unittest.mock import patch
from unittest.mock import PropertyMock, patch

import pytest
from django.urls import reverse
Expand All @@ -11,6 +11,7 @@

from apps.alerts.models import EscalationPolicy
from apps.api.permissions import LegacyAccessControlRole
from apps.api.serializers.schedule_base import ScheduleBaseSerializer
from apps.api.serializers.user import ScheduleUserSerializer
from apps.schedules.models import (
CustomOnCallShift,
Expand All @@ -19,6 +20,7 @@
OnCallScheduleICal,
OnCallScheduleWeb,
)
from apps.slack.models import SlackUserGroup
from common.api_helpers.utils import create_engine_url, serialize_datetime_as_utc_timestamp

ICAL_URL = "https://calendar.google.com/calendar/ical/amixr.io_37gttuakhrtr75ano72p69rt78%40group.calendar.google.com/private-1d00a680ba5be7426c3eb3ef1616e26d/basic.ics"
Expand Down Expand Up @@ -2451,3 +2453,41 @@ def test_team_not_updated_if_not_in_data(

schedule.refresh_from_db()
assert schedule.team == team


@patch.object(SlackUserGroup, "can_be_updated", new_callable=PropertyMock)
@pytest.mark.django_db
def test_can_update_user_groups(
mock_user_group_can_be_updated,
make_organization_and_user_with_plugin_token,
make_slack_team_identity,
make_schedule,
make_slack_user_group,
make_user_auth_headers,
):
mock_user_group_can_be_updated.return_value = True

organization, user, token = make_organization_and_user_with_plugin_token()
slack_team_identity = make_slack_team_identity()
organization.slack_team_identity = slack_team_identity
organization.save()

inactive_user_group = make_slack_user_group(slack_team_identity, is_active=False)
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, user_group=inactive_user_group)

client = APIClient()
url = reverse("api-internal:schedule-detail", kwargs={"pk": schedule.public_primary_key})
response = client.get(url, **make_user_auth_headers(user, token))

assert response.status_code == status.HTTP_200_OK
assert response.json()["warnings"] == [ScheduleBaseSerializer.CANT_UPDATE_USER_GROUP_WARNING]
mock_user_group_can_be_updated.assert_not_called() # should not be called for inactive user group (is_active=False)

active_user_group = make_slack_user_group(slack_team_identity, is_active=True)
schedule.user_group = active_user_group
schedule.save()
response = client.get(url, **make_user_auth_headers(user, token))

assert response.status_code == status.HTTP_200_OK
assert response.json()["warnings"] == []
mock_user_group_can_be_updated.assert_called_once() # should be called for active user group (is_active=True)

0 comments on commit c52d79f

Please sign in to comment.