From 759d83ae473e803db0b3c82f1d85a43e2597c584 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 13 Sep 2023 10:27:26 -0700 Subject: [PATCH] feat(feedback): add organization feedback index GET endpoint (#56065) Closes https://github.com/getsentry/team-replay/issues/175 --- src/sentry/api/urls.py | 7 + .../feedback/endpoints/feedback_ingest.py | 12 +- .../endpoints/organization_feedback_index.py | 35 +++++ .../feedback/migrations/0001_feedback.py | 8 +- src/sentry/feedback/models.py | 5 +- src/sentry/feedback/serializers.py | 53 +++++++ tests/sentry/feedback/test_feedback_ingest.py | 10 +- .../test_organization_feedback_index.py | 143 ++++++++++++++++++ 8 files changed, 265 insertions(+), 8 deletions(-) create mode 100644 src/sentry/feedback/endpoints/organization_feedback_index.py create mode 100644 src/sentry/feedback/serializers.py create mode 100644 tests/sentry/feedback/test_organization_feedback_index.py diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 3899aca54af2ec..3805d2e4f9d3e9 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -36,6 +36,7 @@ DiscoverSavedQueryVisitEndpoint, ) from sentry.feedback.endpoints.feedback_ingest import FeedbackIngestEndpoint +from sentry.feedback.endpoints.organization_feedback_index import OrganizationFeedbackIndexEndpoint from sentry.incidents.endpoints.organization_alert_rule_available_action_index import ( OrganizationAlertRuleAvailableActionIndexEndpoint, ) @@ -1746,6 +1747,12 @@ OrganizationTransactionAnomalyDetectionEndpoint.as_view(), name="sentry-api-0-organization-transaction-anomaly-detection", ), + # Feedback + re_path( + r"^(?P[^\/]+)/feedback/$", + OrganizationFeedbackIndexEndpoint.as_view(), + name="sentry-api-0-organization-feedback-index", + ), # relay usage re_path( r"^(?P[^\/]+)/relay_usage/$", diff --git a/src/sentry/feedback/endpoints/feedback_ingest.py b/src/sentry/feedback/endpoints/feedback_ingest.py index 7fa469c4c5cfb3..719857fd3e07d6 100644 --- a/src/sentry/feedback/endpoints/feedback_ingest.py +++ b/src/sentry/feedback/endpoints/feedback_ingest.py @@ -42,6 +42,9 @@ class FeedbackValidator(serializers.Serializer): request = serializers.JSONField(required=False) tags = serializers.JSONField(required=False) user = serializers.JSONField(required=False) + contexts = serializers.JSONField(required=False) + BrowserContext = serializers.JSONField(required=False) + DeviceContext = serializers.JSONField(required=False) def validate(self, data): try: @@ -56,6 +59,9 @@ def validate(self, data): "user": data.get("user"), "tags": data.get("tags"), "dist": data.get("dist"), + "contexts": data.get("contexts"), + "browser": data.get("BrowserContext"), + "device": data.get("DeviceContext"), } ret["date_added"] = datetime.datetime.fromtimestamp(data["timestamp"]) ret["feedback_id"] = data.get("event_id") or uuid4().hex @@ -63,7 +69,7 @@ def validate(self, data): ret["message"] = data["feedback"]["message"] ret["replay_id"] = data["feedback"].get("replay_id") ret["project_id"] = self.context["project"].id - + ret["organization_id"] = self.context["organization"].id return ret except KeyError: raise serializers.ValidationError("Input has wrong field name or type") @@ -144,7 +150,9 @@ def post(self, request: Request, organization: Organization, project: Project) - ): return Response(status=404) - feedback_validator = FeedbackValidator(data=request.data, context={"project": project}) + feedback_validator = FeedbackValidator( + data=request.data, context={"project": project, "organization": organization} + ) if not feedback_validator.is_valid(): return self.respond(feedback_validator.errors, status=400) diff --git a/src/sentry/feedback/endpoints/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py new file mode 100644 index 00000000000000..1abb4a4ca587f4 --- /dev/null +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -0,0 +1,35 @@ +from __future__ import annotations + +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry import features +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import region_silo_endpoint +from sentry.api.bases.organization import OrganizationEndpoint +from sentry.api.paginator import OffsetPaginator +from sentry.api.serializers.base import serialize +from sentry.feedback.models import Feedback +from sentry.feedback.serializers import FeedbackSerializer +from sentry.models import Organization + + +@region_silo_endpoint +class OrganizationFeedbackIndexEndpoint(OrganizationEndpoint): + publish_status = { + "GET": ApiPublishStatus.EXPERIMENTAL, + } + owner = ApiOwner.FEEDBACK + + def get(self, request: Request, organization: Organization) -> Response: + if not features.has("organizations:user-feedback-ingest", organization, actor=request.user): + return Response(status=404) + + feedback_list = Feedback.objects.filter(organization_id=organization.id) + return self.paginate( + request=request, + queryset=feedback_list, + on_results=lambda x: serialize(x, request.user, FeedbackSerializer()), + paginator_cls=OffsetPaginator, + ) diff --git a/src/sentry/feedback/migrations/0001_feedback.py b/src/sentry/feedback/migrations/0001_feedback.py index 0c06b84d377fa3..742be9b4e87235 100644 --- a/src/sentry/feedback/migrations/0001_feedback.py +++ b/src/sentry/feedback/migrations/0001_feedback.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.20 on 2023-09-07 18:06 +# Generated by Django 3.2.20 on 2023-09-12 23:52 import django.utils.timezone from django.db import migrations, models @@ -38,11 +38,15 @@ class Migration(CheckedMigration): "project_id", sentry.db.models.fields.bounded.BoundedBigIntegerField(db_index=True), ), - ("replay_id", models.CharField(db_index=True, max_length=32, null=True)), + ("replay_id", models.CharField(db_index=True, max_length=100, null=True)), ("url", models.CharField(max_length=1000, null=True)), ("message", models.TextField()), ("feedback_id", sentry.db.models.fields.uuid.UUIDField(max_length=32, unique=True)), ("date_added", models.DateTimeField(default=django.utils.timezone.now)), + ( + "organization_id", + sentry.db.models.fields.bounded.BoundedBigIntegerField(db_index=True), + ), ("data", models.JSONField(null=True)), ], options={ diff --git a/src/sentry/feedback/models.py b/src/sentry/feedback/models.py index 1333c9e56b8221..f3a96a139c4979 100644 --- a/src/sentry/feedback/models.py +++ b/src/sentry/feedback/models.py @@ -11,13 +11,14 @@ class Feedback(Model): __relocation_scope__ = RelocationScope.Excluded project_id = BoundedBigIntegerField(db_index=True) - replay_id = models.CharField(max_length=32, null=True, db_index=True) + replay_id = models.CharField(max_length=100, null=True, db_index=True) url = models.CharField(max_length=1000, null=True) message = models.TextField() feedback_id = UUIDField(unique=True) date_added = models.DateTimeField(default=timezone.now) + organization_id = BoundedBigIntegerField(db_index=True) - # This is the data coming from the Sentry event and includes things like contexts + # This "data" field is the data coming from the Sentry event and includes things like contexts # As we develop the product more, we will add more specific columns and rely on this JSON field less and less data = models.JSONField(null=True) diff --git a/src/sentry/feedback/serializers.py b/src/sentry/feedback/serializers.py new file mode 100644 index 00000000000000..52e1b690122935 --- /dev/null +++ b/src/sentry/feedback/serializers.py @@ -0,0 +1,53 @@ +from typing import Any, Optional, TypedDict + +from sentry.api.serializers.base import Serializer, register +from sentry.feedback.models import Feedback + + +class FeedbackResponseType(TypedDict): + browser: Optional[Any] + locale: Optional[Any] + tags: Optional[Any] + device: Optional[Any] + os: Optional[Any] + user: Optional[Any] + replay_id: Optional[str] + url: Optional[str] + dist: Optional[str] + sdk: Any + contact_email: str + environment: str + feedback_id: str + message: str + platform: str + project_id: str + release: str + status: str + timestamp: str + + +@register(Feedback) +class FeedbackSerializer(Serializer): + def serialize(self, obj, attrs, user, **kwargs) -> FeedbackResponseType: + res: FeedbackResponseType = { + "browser": obj.data.get("browser") or {}, + "locale": obj.data.get("locale") or {}, + "tags": obj.data.get("tags") or {}, + "device": obj.data.get("device") or {}, + "os": obj.data.get("os") or {}, + "user": obj.data.get("user") or {}, + "replay_id": obj.replay_id, + "dist": obj.data.get("dist"), + "sdk": obj.data.get("sdk"), + "contact_email": obj.data.get("feedback").get("contact_email"), + "environment": obj.data.get("environment"), + "feedback_id": obj.feedback_id, + "message": obj.message, + "platform": obj.data.get("platform"), + "project_id": obj.project_id, + "release": obj.data.get("release"), + "status": "unresolved", + "timestamp": obj.date_added, + "url": obj.url, + } + return res diff --git a/tests/sentry/feedback/test_feedback_ingest.py b/tests/sentry/feedback/test_feedback_ingest.py index 29cf9ab7f57ede..49d4d219453c5d 100644 --- a/tests/sentry/feedback/test_feedback_ingest.py +++ b/tests/sentry/feedback/test_feedback_ingest.py @@ -31,6 +31,10 @@ "name": "user", "username": "user2270129", }, + "contexts": { + "BrowserContext": {"name": "Chrome", "version": "116.0.0"}, + "DeviceContext": {"family": "Mac", "model": "Mac", "brand": "Apple", "type": "device"}, + }, } @@ -189,6 +193,8 @@ def test_missing_optional_fields(self): with self.feature({"organizations:user-feedback-ingest": True}): path = reverse(self.endpoint) response = self.client.post( - path, data=test_data_missing_optional_fields, **self.dsn_auth_headers + path, + data=test_data_missing_optional_fields, + **self.dsn_auth_headers, ) - assert response.status_code == 201 + assert response.status_code == 201, response.content diff --git a/tests/sentry/feedback/test_organization_feedback_index.py b/tests/sentry/feedback/test_organization_feedback_index.py new file mode 100644 index 00000000000000..a4e7afbb3d5bfe --- /dev/null +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -0,0 +1,143 @@ +import datetime +import uuid + +from django.urls import reverse +from rest_framework.exceptions import ErrorDetail + +from sentry.feedback.models import Feedback +from sentry.testutils.cases import APITestCase + + +class OrganizationFeedbackIndexTest(APITestCase): + get_endpoint = "sentry-api-0-organization-feedback-index" + post_endpoint = "sentry-api-0-feedback-ingest" + + def setUp(self): + super().setUp() + self.login_as(user=self.user) + + def test_get_feedback_list(self): + # Successful GET + Feedback.objects.create( + data={ + "environment": "production", + "feedback": { + "contact_email": "colton.allen@sentry.io", + "message": "I really like this user-feedback feature!", + "replay_id": "ec3b4dc8b79f417596f7a1aa4fcca5d2", + "url": "https://docs.sentry.io/platforms/javascript/", + }, + "platform": "javascript", + "release": "version@1.3", + "sdk": {"name": "sentry.javascript.react", "version": "6.18.1"}, + "tags": {"key": "value"}, + "user": { + "email": "username@example.com", + "id": "123", + "ip_address": "127.0.0.1", + "name": "user", + "username": "user2270129", + }, + "dist": "abc123", + "contexts": {}, + }, + date_added=datetime.datetime.fromtimestamp(1234456), + feedback_id=uuid.UUID("1ffe0775ac0f4417aed9de36d9f6f8dc"), + url="https://docs.sentry.io/platforms/javascript/", + message="I really like this user-feedback feature!", + replay_id=uuid.UUID("ec3b4dc8b79f417596f7a1aa4fcca5d2"), + project_id=self.project.id, + organization_id=self.organization.id, + ) + + Feedback.objects.create( + data={ + "environment": "prod", + "feedback": { + "contact_email": "michelle.zhang@sentry.io", + "message": "I also really like this user-feedback feature!", + "replay_id": "zc3b5xy8b79f417596f7a1tt4fffa5d2", + "url": "https://docs.sentry.io/platforms/electron/", + }, + "platform": "electron", + "release": "version@1.3", + "request": { + "headers": { + "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36" + } + }, + "sdk": {"name": "sentry.javascript.react", "version": "5.18.1"}, + "tags": {"key": "value"}, + "user": { + "email": "username@example.com", + "id": "123", + "ip_address": "127.0.0.1", + "name": "user", + "username": "user2270129", + }, + "dist": "abc123", + "contexts": {}, + }, + date_added=datetime.datetime.fromtimestamp(12344100333), + feedback_id=uuid.UUID("2ffe0775ac0f4417aed9de36d9f6f8ab"), + url="https://docs.sentry.io/platforms/electron/", + message="I also really like this user-feedback feature!", + replay_id=uuid.UUID("ec3b4dc8b79f417596f7a1aa4fcca5d2"), + project_id=self.project.id, + organization_id=self.organization.id, + ) + + with self.feature({"organizations:user-feedback-ingest": True}): + path = reverse(self.get_endpoint, args=[self.organization.slug]) + response = self.client.get(path) + assert response.status_code == 200 + + # Should get what we have in the database + assert len(response.data) == 2 + # Test first item + feedback = response.data[0] + assert feedback["dist"] == "abc123" + assert feedback["url"] == "https://docs.sentry.io/platforms/javascript/" + assert feedback["message"] == "I really like this user-feedback feature!" + assert feedback["feedback_id"] == uuid.UUID("1ffe0775ac0f4417aed9de36d9f6f8dc") + assert feedback["platform"] == "javascript" + assert feedback["sdk"]["name"] == "sentry.javascript.react" + assert feedback["tags"]["key"] == "value" + assert feedback["contact_email"] == "colton.allen@sentry.io" + + # Test second item + feedback = response.data[1] + assert feedback["environment"] == "prod" + assert feedback["url"] == "https://docs.sentry.io/platforms/electron/" + assert feedback["message"] == "I also really like this user-feedback feature!" + assert feedback["feedback_id"] == uuid.UUID("2ffe0775ac0f4417aed9de36d9f6f8ab") + assert feedback["platform"] == "electron" + assert feedback["sdk"]["name"] == "sentry.javascript.react" + assert feedback["sdk"]["version"] == "5.18.1" + + # Testing GET parameters + # For now, only testing per_page; others (such as sort, query) are not well-defined or not necessary for MVP + response = self.client.get( + path=path, + data={"per_page": 1}, + content_type="application/json", + ) + assert response.status_code == 200 + assert len(response.data) == 1 + + def test_no_feature_enabled(self): + # Unsuccessful GET + with self.feature({"organizations:user-feedback-ingest": False}): + path = reverse(self.get_endpoint, args=[self.organization.slug]) + get_response = self.client.get(path) + assert get_response.status_code == 404 + + def test_bad_slug_path(self): + # Bad slug in path should lead to unsuccessful GET + with self.feature({"organizations:user-feedback-ingest": True}): + path = reverse(self.get_endpoint, args=["testslug123345"]) + get_response = self.client.get(path) + assert get_response.status_code == 404 + assert get_response.data == { + "detail": ErrorDetail(string="The requested resource does not exist", code="error") + }