diff --git a/CHANGELOG.md b/CHANGELOG.md index fb9f025aa7..5b3729149c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ 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.3.87 (2024-01-15) + +### Fixed + +- Fix occasional `AttributeError` in `apps.grafana_plugin.tasks.sync.sync_organization_async` task by @joeyorlando ([#3687](https://github.com/grafana/oncall/pull/3687)) + ## v1.3.86 (2024-01-12) ### Fixed diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index 4544f5b9b8..142a28edbc 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -196,7 +196,7 @@ class GrafanaServiceAccountToken(typing.TypedDict): class PluginSettings(typing.TypedDict): enabled: bool - jsonData: typing.Dict[str, str] + jsonData: typing.NotRequired[typing.Dict[str, str]] class TeamsResponse(_BaseGrafanaAPIResponse): teams: typing.List["GrafanaAPIClient.Types.GrafanaTeam"] diff --git a/engine/apps/user_management/sync.py b/engine/apps/user_management/sync.py index 94be2ebc8c..d3bd1d28b2 100644 --- a/engine/apps/user_management/sync.py +++ b/engine/apps/user_management/sync.py @@ -54,7 +54,7 @@ def _sync_organization(organization: Organization) -> None: grafana_incident_settings, _ = grafana_api_client.get_grafana_incident_plugin_settings() if grafana_incident_settings is not None: organization.is_grafana_incident_enabled = grafana_incident_settings["enabled"] - organization.grafana_incident_backend_url = grafana_incident_settings["jsonData"].get( + organization.grafana_incident_backend_url = grafana_incident_settings.get("jsonData", {}).get( GrafanaAPIClient.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY ) else: diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 20013ba9ad..6c0a6d9253 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -177,6 +177,14 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make @pytest.mark.django_db +@pytest.mark.parametrize( + "get_grafana_incident_plugin_settings_return_value", + [ + ({"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, None), + # missing jsonData (sometimes this is what we get back from the Grafana API) + ({"enabled": True}, None), + ], +) @patch.object(GrafanaAPIClient, "is_rbac_enabled_for_organization", return_value=False) @patch.object( GrafanaAPIClient, @@ -212,21 +220,20 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make ), ) @patch.object(GrafanaAPIClient, "check_token", return_value=(None, {"connected": True})) -@patch.object( - GrafanaAPIClient, - "get_grafana_incident_plugin_settings", - return_value=({"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, None), -) +@patch.object(GrafanaAPIClient, "get_grafana_incident_plugin_settings") @patch("apps.user_management.sync.org_sync_signal") def test_sync_organization( mocked_org_sync_signal, - _mock_get_grafana_incident_plugin_settings, + mock_get_grafana_incident_plugin_settings, _mock_check_token, _mock_get_teams, _mock_get_users, _mock_is_rbac_enabled_for_organization, + get_grafana_incident_plugin_settings_return_value, make_organization, ): + mock_get_grafana_incident_plugin_settings.return_value = get_grafana_incident_plugin_settings_return_value + organization = make_organization() api_members_response = ( @@ -259,7 +266,10 @@ def test_sync_organization( # check that is_grafana_incident_enabled flag is set assert organization.is_grafana_incident_enabled is True - assert organization.grafana_incident_backend_url == MOCK_GRAFANA_INCIDENT_BACKEND_URL + if get_grafana_incident_plugin_settings_return_value[0].get("jsonData"): + assert organization.grafana_incident_backend_url == MOCK_GRAFANA_INCIDENT_BACKEND_URL + else: + assert organization.grafana_incident_backend_url is None mocked_org_sync_signal.send.assert_called_once_with(sender=None, organization=organization)