From f40e0463d2b3ee324b76569ae78ae1cf6fc30d17 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 16 Oct 2023 11:38:08 -0600 Subject: [PATCH] Fix inconsistency with API URL returned by plugin status (#3122) # What this PR does Change status to return backend URL in the same way (trailing /) that is used to build URLs throughout by using create_engine_url utility function. ## Which issue(s) this PR fixes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 ++++ .../apps/grafana_plugin/tests/test_status.py | 19 ++++++++++++++++++- engine/apps/grafana_plugin/views/status.py | 5 +++-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2960660..0ed432e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Update plugin OnCaller role permissions ([#3145](https://github.com/grafana/oncall/pull/3145)) +### Fixed + +- Fix plugin status to always return URL with trailing / @mderynck ([#3122](https://github.com/grafana/oncall/pull/3122)) + ## v1.3.43 (2023-10-05) ### Added diff --git a/engine/apps/grafana_plugin/tests/test_status.py b/engine/apps/grafana_plugin/tests/test_status.py index 24374f56..805a7ff0 100644 --- a/engine/apps/grafana_plugin/tests/test_status.py +++ b/engine/apps/grafana_plugin/tests/test_status.py @@ -10,9 +10,11 @@ GRAFANA_TOKEN = "TESTTOKEN" GRAFANA_URL = "hello.com" LICENSE = "asdfasdf" VERSION = "asdfasdfasdf" -BASE_URL = "http://asdasdqweqweqw.com/oncall" +BASE_URL = "http://asdasdqweqweqw.com/oncall/" +BASE_URL_NO_TRAILING_SLASH = "http://asdasdqweqweqw.com/oncall" GRAFANA_CONTEXT_DATA = {"IsAnonymous": False} SETTINGS = {"LICENSE": LICENSE, "VERSION": VERSION, "BASE_URL": BASE_URL} +SETTINGS_ALT = SETTINGS | {"BASE_URL": BASE_URL_NO_TRAILING_SLASH} def _check_status_response(auth_headers, client): @@ -86,3 +88,18 @@ def test_status_mobile_app_auth_token( auth_headers = {"HTTP_AUTHORIZATION": f"{auth_token}"} _check_status_response(auth_headers, client) + + +@pytest.mark.django_db +@override_settings(**SETTINGS_ALT) +def test_status_base_url_trailing_slash(make_organization_and_user_with_plugin_token, make_user_auth_headers): + organization, user, token = make_organization_and_user_with_plugin_token() + organization.grafana_url = GRAFANA_URL + organization.api_token_status = Organization.API_TOKEN_STATUS_OK + organization.save(update_fields=["grafana_url", "api_token_status"]) + + client = APIClient() + auth_headers = make_user_auth_headers( + user, token, grafana_token=GRAFANA_TOKEN, grafana_context_data=GRAFANA_CONTEXT_DATA + ) + _check_status_response(auth_headers, client) diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index 9b5d3a21..340c1774 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -9,6 +9,7 @@ from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Organization from common.api_helpers.mixins import GrafanaHeadersMixin +from common.api_helpers.utils import create_engine_url class StatusView(GrafanaHeadersMixin, APIView): @@ -35,14 +36,14 @@ class StatusView(GrafanaHeadersMixin, APIView): is_installed = False token_ok = False allow_signup = True - api_url = settings.BASE_URL + api_url = create_engine_url("") # Check if organization is in OnCall database if organization: is_installed = True token_ok = organization.api_token_status == Organization.API_TOKEN_STATUS_OK if organization.is_moved: - api_url = organization.migration_destination.oncall_backend_url + api_url = create_engine_url("", override_base=organization.migration_destination.oncall_backend_url) else: allow_signup = DynamicSetting.objects.get_or_create( name="allow_plugin_organization_signup", defaults={"boolean_value": True}