From 782dd26457dd4310c3c1cebee1c66850266f24c0 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:53:29 -0700 Subject: [PATCH 01/11] fix(replays api): change limit to be per_page --- src/sentry/replays/blueprints/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/blueprints/api.md b/src/sentry/replays/blueprints/api.md index 06a11c909a6aff..540f500d1adac8 100644 --- a/src/sentry/replays/blueprints/api.md +++ b/src/sentry/replays/blueprints/api.md @@ -260,7 +260,7 @@ Deletes a replay instance. - w - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) - end (optional, string) - ISO 8601 format. Required if `start` is set. - - limit (optional, number) + - per_page (optional, number) Default: 10 - offset (optional, number) Default: 0 From f8b0a6f34bb60581f36ef41e09923263bedf7965 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:55:27 -0700 Subject: [PATCH 02/11] Revert "fix(replays api): change limit to be per_page" This reverts commit 782dd26457dd4310c3c1cebee1c66850266f24c0. --- src/sentry/replays/blueprints/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/replays/blueprints/api.md b/src/sentry/replays/blueprints/api.md index 540f500d1adac8..06a11c909a6aff 100644 --- a/src/sentry/replays/blueprints/api.md +++ b/src/sentry/replays/blueprints/api.md @@ -260,7 +260,7 @@ Deletes a replay instance. - w - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) - end (optional, string) - ISO 8601 format. Required if `start` is set. - - per_page (optional, number) + - limit (optional, number) Default: 10 - offset (optional, number) Default: 0 From 8aaaf4fafadd9b0645289d8cbdbc1e70495cdee2 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Mon, 11 Sep 2023 19:28:37 -0700 Subject: [PATCH 03/11] feat(feedback): add organization feedback index GET endpoint --- src/sentry/api/urls.py | 7 ++ .../endpoints/organization_feedback_index.py | 103 ++++++++++++++++++ src/sentry/feedback/serializers.py | 29 +++++ .../test_organization_feedback_index.py | 86 +++++++++++++++ 4 files changed, 225 insertions(+) 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/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py new file mode 100644 index 00000000000000..87691e06c4edce --- /dev/null +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -0,0 +1,103 @@ +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.authentication import ( + ApiKeyAuthentication, + DSNAuthentication, + OrgAuthTokenAuthentication, + TokenAuthentication, +) +from sentry.api.base import Endpoint, region_silo_endpoint +from sentry.api.bases.project import ProjectPermission +from sentry.api.exceptions import ResourceDoesNotExist +from sentry.api.serializers.base import serialize +from sentry.constants import ObjectStatus +from sentry.feedback.models import Feedback +from sentry.feedback.serializers import FeedbackSerializer +from sentry.models import Organization, ProjectKey +from sentry.models.project import Project +from sentry.utils.sdk import bind_organization_context, configure_scope + + +class OrganizationFeedbackIndexPermission(ProjectPermission): + scope_map = { + "GET": ["project:read", "project:write", "project:admin"], + } + + +@region_silo_endpoint +class OrganizationFeedbackIndexEndpoint(Endpoint): + publish_status = { + "GET": ApiPublishStatus.EXPERIMENTAL, + } + owner = ApiOwner.FEEDBACK + + # Authentication code borrowed from the monitor endpoints (which will eventually be removed) + authentication_classes = ( + DSNAuthentication, + TokenAuthentication, + OrgAuthTokenAuthentication, + ApiKeyAuthentication, + ) + + permission_classes = (OrganizationFeedbackIndexPermission,) + + def convert_args( + self, + request: Request, + organization_slug: str | None = None, + *args, + **kwargs, + ): + using_dsn_auth = isinstance(request.auth, ProjectKey) + + # When using DSN auth we're able to infer the organization slug + if not organization_slug and using_dsn_auth: + organization_slug = request.auth.project.organization.slug # type: ignore + + if organization_slug: + try: + organization = Organization.objects.get_from_cache(slug=organization_slug) + # Try lookup by slug first. This requires organization context since + # slugs are unique only to the organization + except (Organization.DoesNotExist): + raise ResourceDoesNotExist + + project = request.auth.project # type: ignore + + if project.status != ObjectStatus.ACTIVE: + raise ResourceDoesNotExist + + if using_dsn_auth and project.id != request.auth.project_id: # type: ignore + raise ResourceDoesNotExist + + if organization_slug and project.organization.slug != organization_slug: + raise ResourceDoesNotExist + + # Check project permission. Required for Token style authentication + self.check_object_permissions(request, project) + + with configure_scope() as scope: + scope.set_tag("project", project.id) + + bind_organization_context(project.organization) + + request._request.organization = project.organization # type: ignore + + kwargs["organization"] = organization + kwargs["project"] = project + return args, kwargs + + def get(self, request: Request, organization: Organization, project: Project) -> Response: + if not features.has( + "organizations:user-feedback-ingest", project.organization, actor=request.user + ): + return Response(status=404) + + feedback_list = Feedback.objects.filter(project_id=project.id) + return Response(serialize(list(feedback_list), request.user, FeedbackSerializer())) diff --git a/src/sentry/feedback/serializers.py b/src/sentry/feedback/serializers.py new file mode 100644 index 00000000000000..6c1b89158351a1 --- /dev/null +++ b/src/sentry/feedback/serializers.py @@ -0,0 +1,29 @@ +from typing import Any, Optional, TypedDict + +from sentry.api.serializers.base import Serializer, register +from sentry.feedback.models import Feedback + + +class FeedbackResponseType(TypedDict): + date_added: str + replay_id: Optional[str] + project_id: str + url: Optional[str] + message: str + data: Optional[Any] + feedback_id: str + + +@register(Feedback) +class FeedbackSerializer(Serializer): + def serialize(self, obj, attrs, user, **kwargs) -> FeedbackResponseType: + res: FeedbackResponseType = { + "date_added": obj.date_added, + "replay_id": obj.replay_id, + "url": obj.url, + "message": obj.message, + "data": obj.data, + "feedback_id": obj.feedback_id, + "project_id": obj.project_id, + } + return res 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..8a122a3c0b7671 --- /dev/null +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -0,0 +1,86 @@ +import uuid + +from django.urls import reverse + +from sentry.testutils.cases import MonitorIngestTestCase + +post_data = { + "dist": "abc123", + "environment": "production", + "event_id": "1ffe0775ac0f4417aed9de36d9f6f8dc", + "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", + "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": "6.18.1"}, + "tags": {"key": "value"}, + "timestamp": 1234456, + "user": { + "email": "username@example.com", + "id": "123", + "ip_address": "127.0.0.1", + "name": "user", + "username": "user2270129", + }, +} + + +class OrganizationFeedbackIndexTest(MonitorIngestTestCase): + get_endpoint = "sentry-api-0-organization-feedback-index" + post_endpoint = "sentry-api-0-feedback-ingest" + + def test_get_feedback_list(self): + # Successful GET + with self.feature({"organizations:user-feedback-ingest": True}): + get_path = reverse(self.get_endpoint, args=[self.organization.slug]) + post_path = reverse(self.post_endpoint) + self.client.post( + post_path, + data=post_data, + **self.dsn_auth_headers, + ) + get_response = self.client.get(get_path, **self.dsn_auth_headers) + assert get_response.status_code == 200 + + # Should get what we just posted + assert len(get_response.data) == 1 + feedback = get_response.data[0] + assert feedback["data"]["dist"] == "abc123" + assert feedback["data"]["environment"] == "production" + assert ( + feedback["data"]["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["data"]["platform"] == "javascript" + assert feedback["data"]["sdk"]["name"] == "sentry.javascript.react" + assert feedback["data"]["tags"]["key"] == "value" + assert feedback["data"]["user"]["email"] == "username@example.com" + + def test_no_feature_enabled(self): + # Unsuccessful GET + with self.feature({"organizations:user-feedback-ingest": False}): + get_path = reverse(self.get_endpoint, args=[self.organization.slug]) + post_path = reverse(self.post_endpoint) + self.client.post( + post_path, + data=post_data, + **self.dsn_auth_headers, + ) + get_response = self.client.get(get_path, **self.dsn_auth_headers) + assert get_response.status_code == 404 + + # blueprint: how are we getting attributes like OS, browser, device, locale, etc? + # TODO: test parameters + # TODO: test authorization failure + # TODO: test invalid org From 76c019d2fc400e0ccf1cd6271c75f7048ab72695 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 12 Sep 2023 11:32:33 -0700 Subject: [PATCH 04/11] add tests --- .../endpoints/organization_feedback_index.py | 2 +- .../test_organization_feedback_index.py | 84 +++++++++++++++++-- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/sentry/feedback/endpoints/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py index 87691e06c4edce..321c0bc5b0bb26 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_index.py +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -99,5 +99,5 @@ def get(self, request: Request, organization: Organization, project: Project) -> ): return Response(status=404) - feedback_list = Feedback.objects.filter(project_id=project.id) + feedback_list = Feedback.objects.filter(project_id=project.id) # should this filter? return Response(serialize(list(feedback_list), request.user, FeedbackSerializer())) diff --git a/tests/sentry/feedback/test_organization_feedback_index.py b/tests/sentry/feedback/test_organization_feedback_index.py index 8a122a3c0b7671..c4ceb5ebe28e98 100644 --- a/tests/sentry/feedback/test_organization_feedback_index.py +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -1,10 +1,11 @@ import uuid from django.urls import reverse +from rest_framework.exceptions import ErrorDetail from sentry.testutils.cases import MonitorIngestTestCase -post_data = { +post_data_1 = { "dist": "abc123", "environment": "production", "event_id": "1ffe0775ac0f4417aed9de36d9f6f8dc", @@ -33,6 +34,26 @@ }, } +post_data_2 = { + "environment": "prod", + "event_id": "2ffe0775ac0f4417aed9de36d9f6f8ab", + "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"}, + "timestamp": 12344100333, +} + class OrganizationFeedbackIndexTest(MonitorIngestTestCase): get_endpoint = "sentry-api-0-organization-feedback-index" @@ -45,17 +66,22 @@ def test_get_feedback_list(self): post_path = reverse(self.post_endpoint) self.client.post( post_path, - data=post_data, + data=post_data_1, + **self.dsn_auth_headers, + ) + self.client.post( + post_path, + data=post_data_2, **self.dsn_auth_headers, ) get_response = self.client.get(get_path, **self.dsn_auth_headers) assert get_response.status_code == 200 # Should get what we just posted - assert len(get_response.data) == 1 + assert len(get_response.data) == 2 + # Test first item feedback = get_response.data[0] assert feedback["data"]["dist"] == "abc123" - assert feedback["data"]["environment"] == "production" assert ( feedback["data"]["feedback"]["url"] == "https://docs.sentry.io/platforms/javascript/" @@ -67,6 +93,18 @@ def test_get_feedback_list(self): assert feedback["data"]["tags"]["key"] == "value" assert feedback["data"]["user"]["email"] == "username@example.com" + # Test second item + feedback = get_response.data[1] + assert feedback["data"]["environment"] == "prod" + assert ( + feedback["data"]["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["data"]["platform"] == "electron" + assert feedback["data"]["sdk"]["name"] == "sentry.javascript.react" + assert feedback["data"]["sdk"]["version"] == "5.18.1" + def test_no_feature_enabled(self): # Unsuccessful GET with self.feature({"organizations:user-feedback-ingest": False}): @@ -74,13 +112,41 @@ def test_no_feature_enabled(self): post_path = reverse(self.post_endpoint) self.client.post( post_path, - data=post_data, + data=post_data_1, **self.dsn_auth_headers, ) get_response = self.client.get(get_path, **self.dsn_auth_headers) assert get_response.status_code == 404 - # blueprint: how are we getting attributes like OS, browser, device, locale, etc? - # TODO: test parameters - # TODO: test authorization failure - # TODO: test invalid org + def test_no_authorization(self): + # No authorization should lead to unsuccessful GET + with self.feature({"organizations:user-feedback-ingest": False}): + get_path = reverse(self.get_endpoint, args=[self.organization.slug]) + post_path = reverse(self.post_endpoint) + self.client.post( + post_path, + data=post_data_1, + **self.dsn_auth_headers, + ) + get_response = self.client.get(get_path) + assert get_response.status_code == 401 + assert get_response.data == {"detail": "Authentication credentials were not provided."} + + def test_bad_slug_path(self): + # Bad slug in path should lead to unsuccessful GET + with self.feature({"organizations:user-feedback-ingest": True}): + get_path = reverse(self.get_endpoint, args=["testslug123345"]) + post_path = reverse(self.post_endpoint) + self.client.post( + post_path, + data=post_data_1, + **self.dsn_auth_headers, + ) + get_response = self.client.get(get_path, **self.dsn_auth_headers) + assert get_response.status_code == 404 + assert get_response.data == { + "detail": ErrorDetail(string="The requested resource does not exist", code="error") + } + + +# TODO: test request parameters (e.g. sort, query) From 02444ccf54f6df311d0ab13fcf592555981cd9c3 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 12 Sep 2023 16:12:26 -0700 Subject: [PATCH 05/11] update model with organization_id stored and add per_page for endpoint --- .../feedback/endpoints/feedback_ingest.py | 8 ++- .../endpoints/organization_feedback_index.py | 10 +++- .../feedback/migrations/0001_feedback.py | 7 ++- src/sentry/feedback/models.py | 3 +- src/sentry/feedback/serializers.py | 41 ++++++++++--- tests/sentry/feedback/test_feedback_ingest.py | 10 +++- .../test_organization_feedback_index.py | 57 ++++++++++++------- 7 files changed, 100 insertions(+), 36 deletions(-) diff --git a/src/sentry/feedback/endpoints/feedback_ingest.py b/src/sentry/feedback/endpoints/feedback_ingest.py index 7fa469c4c5cfb3..5f5d7161809af7 100644 --- a/src/sentry/feedback/endpoints/feedback_ingest.py +++ b/src/sentry/feedback/endpoints/feedback_ingest.py @@ -42,6 +42,7 @@ class FeedbackValidator(serializers.Serializer): request = serializers.JSONField(required=False) tags = serializers.JSONField(required=False) user = serializers.JSONField(required=False) + contexts = serializers.JSONField(required=False) def validate(self, data): try: @@ -56,6 +57,7 @@ def validate(self, data): "user": data.get("user"), "tags": data.get("tags"), "dist": data.get("dist"), + "contexts": data.get("contexts"), } ret["date_added"] = datetime.datetime.fromtimestamp(data["timestamp"]) ret["feedback_id"] = data.get("event_id") or uuid4().hex @@ -63,7 +65,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 +146,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 index 321c0bc5b0bb26..d7d2628d5284d0 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_index.py +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -15,6 +15,7 @@ from sentry.api.base import Endpoint, region_silo_endpoint from sentry.api.bases.project import ProjectPermission from sentry.api.exceptions import ResourceDoesNotExist +from sentry.api.paginator import OffsetPaginator from sentry.api.serializers.base import serialize from sentry.constants import ObjectStatus from sentry.feedback.models import Feedback @@ -99,5 +100,10 @@ def get(self, request: Request, organization: Organization, project: Project) -> ): return Response(status=404) - feedback_list = Feedback.objects.filter(project_id=project.id) # should this filter? - return Response(serialize(list(feedback_list), request.user, FeedbackSerializer())) + feedback_list = Feedback.objects.filter(project_id=project.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..82508bb1899cb8 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:03 import django.utils.timezone from django.db import migrations, models @@ -20,6 +20,7 @@ class Migration(CheckedMigration): # have ops run this and not block the deploy. Note that while adding an index is a schema # change, it's completely safe to run the operation after the code has deployed. is_dangerous = False + initial = True dependencies = [] @@ -43,6 +44,10 @@ class Migration(CheckedMigration): ("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..c98e3459666554 100644 --- a/src/sentry/feedback/models.py +++ b/src/sentry/feedback/models.py @@ -16,8 +16,9 @@ class Feedback(Model): 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 index 6c1b89158351a1..42f848fc05ff70 100644 --- a/src/sentry/feedback/serializers.py +++ b/src/sentry/feedback/serializers.py @@ -5,25 +5,50 @@ class FeedbackResponseType(TypedDict): - date_added: str + browser: Optional[Any] + locale: Optional[Any] + tags: Optional[Any] + device: Optional[Any] + os: Optional[Any] + user: Optional[Any] replay_id: Optional[str] - project_id: str url: Optional[str] + dist: Optional[str] + sdk: Any + contact_email: str + environment: str + id: str message: str - data: Optional[Any] - feedback_id: str + platform: str + project_id: str + release: str + status: str + timestamp: str + url: str @register(Feedback) class FeedbackSerializer(Serializer): def serialize(self, obj, attrs, user, **kwargs) -> FeedbackResponseType: res: FeedbackResponseType = { - "date_added": obj.date_added, + "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, - "url": obj.url, + "dist": obj.data.get("dist"), + "sdk": obj.data.get("sdk"), + "contact_email": obj.data.get("feedback").get("contact_email"), + "environment": obj.data.get("environment"), + "id": obj.feedback_id, "message": obj.message, - "data": obj.data, - "feedback_id": obj.feedback_id, + "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 index c4ceb5ebe28e98..707d65d532907c 100644 --- a/tests/sentry/feedback/test_organization_feedback_index.py +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -81,29 +81,24 @@ def test_get_feedback_list(self): assert len(get_response.data) == 2 # Test first item feedback = get_response.data[0] - assert feedback["data"]["dist"] == "abc123" - assert ( - feedback["data"]["feedback"]["url"] - == "https://docs.sentry.io/platforms/javascript/" - ) + 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["data"]["platform"] == "javascript" - assert feedback["data"]["sdk"]["name"] == "sentry.javascript.react" - assert feedback["data"]["tags"]["key"] == "value" - assert feedback["data"]["user"]["email"] == "username@example.com" + assert 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 = get_response.data[1] - assert feedback["data"]["environment"] == "prod" - assert ( - feedback["data"]["feedback"]["url"] == "https://docs.sentry.io/platforms/electron/" - ) + 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["data"]["platform"] == "electron" - assert feedback["data"]["sdk"]["name"] == "sentry.javascript.react" - assert feedback["data"]["sdk"]["version"] == "5.18.1" + assert feedback["id"] == uuid.UUID("2ffe0775ac0f4417aed9de36d9f6f8ab") + assert feedback["platform"] == "electron" + assert feedback["sdk"]["name"] == "sentry.javascript.react" + assert feedback["sdk"]["version"] == "5.18.1" def test_no_feature_enabled(self): # Unsuccessful GET @@ -148,5 +143,27 @@ def test_bad_slug_path(self): "detail": ErrorDetail(string="The requested resource does not exist", code="error") } - -# TODO: test request parameters (e.g. sort, query) + def test_parameters(self): + # Testing GET parameters + # For now, only testing per_page; others (such as sort, query) are not well-defined or not necessary for MVP + with self.feature({"organizations:user-feedback-ingest": True}): + get_path = reverse(self.get_endpoint, args=[self.organization.slug]) + post_path = reverse(self.post_endpoint) + self.client.post( + post_path, + data=post_data_1, + **self.dsn_auth_headers, + ) + self.client.post( + post_path, + data=post_data_2, + **self.dsn_auth_headers, + ) + get_response = self.client.get( + path=get_path, + data={"per_page": 1}, + content_type="application/json", + **self.dsn_auth_headers, + ) + assert get_response.status_code == 200 + assert len(get_response.data) == 1 From 8ffe896b847024d4a60369dc87dff472efa049e1 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 12 Sep 2023 16:54:32 -0700 Subject: [PATCH 06/11] update endpoint to inherit from OrganizationEndpoint --- .../feedback/endpoints/feedback_ingest.py | 4 + .../endpoints/organization_feedback_index.py | 81 +------ .../feedback/migrations/0001_feedback.py | 5 +- src/sentry/feedback/models.py | 2 +- .../test_organization_feedback_index.py | 222 ++++++++---------- 5 files changed, 112 insertions(+), 202 deletions(-) diff --git a/src/sentry/feedback/endpoints/feedback_ingest.py b/src/sentry/feedback/endpoints/feedback_ingest.py index 5f5d7161809af7..719857fd3e07d6 100644 --- a/src/sentry/feedback/endpoints/feedback_ingest.py +++ b/src/sentry/feedback/endpoints/feedback_ingest.py @@ -43,6 +43,8 @@ class FeedbackValidator(serializers.Serializer): 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: @@ -58,6 +60,8 @@ def validate(self, data): "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 diff --git a/src/sentry/feedback/endpoints/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py index d7d2628d5284d0..b172bd7689de68 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_index.py +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -6,23 +6,14 @@ from sentry import features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.authentication import ( - ApiKeyAuthentication, - DSNAuthentication, - OrgAuthTokenAuthentication, - TokenAuthentication, -) -from sentry.api.base import Endpoint, region_silo_endpoint +from sentry.api.base import region_silo_endpoint +from sentry.api.bases.organization import OrganizationEndpoint from sentry.api.bases.project import ProjectPermission -from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.paginator import OffsetPaginator from sentry.api.serializers.base import serialize -from sentry.constants import ObjectStatus from sentry.feedback.models import Feedback from sentry.feedback.serializers import FeedbackSerializer -from sentry.models import Organization, ProjectKey -from sentry.models.project import Project -from sentry.utils.sdk import bind_organization_context, configure_scope +from sentry.models import Organization class OrganizationFeedbackIndexPermission(ProjectPermission): @@ -32,75 +23,17 @@ class OrganizationFeedbackIndexPermission(ProjectPermission): @region_silo_endpoint -class OrganizationFeedbackIndexEndpoint(Endpoint): +class OrganizationFeedbackIndexEndpoint(OrganizationEndpoint): publish_status = { "GET": ApiPublishStatus.EXPERIMENTAL, } owner = ApiOwner.FEEDBACK - # Authentication code borrowed from the monitor endpoints (which will eventually be removed) - authentication_classes = ( - DSNAuthentication, - TokenAuthentication, - OrgAuthTokenAuthentication, - ApiKeyAuthentication, - ) - - permission_classes = (OrganizationFeedbackIndexPermission,) - - def convert_args( - self, - request: Request, - organization_slug: str | None = None, - *args, - **kwargs, - ): - using_dsn_auth = isinstance(request.auth, ProjectKey) - - # When using DSN auth we're able to infer the organization slug - if not organization_slug and using_dsn_auth: - organization_slug = request.auth.project.organization.slug # type: ignore - - if organization_slug: - try: - organization = Organization.objects.get_from_cache(slug=organization_slug) - # Try lookup by slug first. This requires organization context since - # slugs are unique only to the organization - except (Organization.DoesNotExist): - raise ResourceDoesNotExist - - project = request.auth.project # type: ignore - - if project.status != ObjectStatus.ACTIVE: - raise ResourceDoesNotExist - - if using_dsn_auth and project.id != request.auth.project_id: # type: ignore - raise ResourceDoesNotExist - - if organization_slug and project.organization.slug != organization_slug: - raise ResourceDoesNotExist - - # Check project permission. Required for Token style authentication - self.check_object_permissions(request, project) - - with configure_scope() as scope: - scope.set_tag("project", project.id) - - bind_organization_context(project.organization) - - request._request.organization = project.organization # type: ignore - - kwargs["organization"] = organization - kwargs["project"] = project - return args, kwargs - - def get(self, request: Request, organization: Organization, project: Project) -> Response: - if not features.has( - "organizations:user-feedback-ingest", project.organization, actor=request.user - ): + 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(project_id=project.id) + feedback_list = Feedback.objects.filter(project_id__in=organization.project_set.all()) return self.paginate( request=request, queryset=feedback_list, diff --git a/src/sentry/feedback/migrations/0001_feedback.py b/src/sentry/feedback/migrations/0001_feedback.py index 82508bb1899cb8..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-12 23:03 +# Generated by Django 3.2.20 on 2023-09-12 23:52 import django.utils.timezone from django.db import migrations, models @@ -20,7 +20,6 @@ class Migration(CheckedMigration): # have ops run this and not block the deploy. Note that while adding an index is a schema # change, it's completely safe to run the operation after the code has deployed. is_dangerous = False - initial = True dependencies = [] @@ -39,7 +38,7 @@ 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)), diff --git a/src/sentry/feedback/models.py b/src/sentry/feedback/models.py index c98e3459666554..f3a96a139c4979 100644 --- a/src/sentry/feedback/models.py +++ b/src/sentry/feedback/models.py @@ -11,7 +11,7 @@ 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) diff --git a/tests/sentry/feedback/test_organization_feedback_index.py b/tests/sentry/feedback/test_organization_feedback_index.py index 707d65d532907c..ac9ef1a71d4ec2 100644 --- a/tests/sentry/feedback/test_organization_feedback_index.py +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -1,86 +1,101 @@ +import datetime import uuid from django.urls import reverse from rest_framework.exceptions import ErrorDetail -from sentry.testutils.cases import MonitorIngestTestCase +from sentry.feedback.models import Feedback +from sentry.testutils.cases import APITestCase -post_data_1 = { - "dist": "abc123", - "environment": "production", - "event_id": "1ffe0775ac0f4417aed9de36d9f6f8dc", - "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", - "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": "6.18.1"}, - "tags": {"key": "value"}, - "timestamp": 1234456, - "user": { - "email": "username@example.com", - "id": "123", - "ip_address": "127.0.0.1", - "name": "user", - "username": "user2270129", - }, -} -post_data_2 = { - "environment": "prod", - "event_id": "2ffe0775ac0f4417aed9de36d9f6f8ab", - "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"}, - "timestamp": 12344100333, -} - - -class OrganizationFeedbackIndexTest(MonitorIngestTestCase): +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}): - get_path = reverse(self.get_endpoint, args=[self.organization.slug]) - post_path = reverse(self.post_endpoint) - self.client.post( - post_path, - data=post_data_1, - **self.dsn_auth_headers, - ) - self.client.post( - post_path, - data=post_data_2, - **self.dsn_auth_headers, - ) - get_response = self.client.get(get_path, **self.dsn_auth_headers) - assert get_response.status_code == 200 + path = reverse(self.get_endpoint, args=[self.organization.slug]) + response = self.client.get(path) + assert response.status_code == 200 - # Should get what we just posted - assert len(get_response.data) == 2 + # Should get what we have in the database + assert len(response.data) == 2 # Test first item - feedback = get_response.data[0] + 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!" @@ -91,7 +106,7 @@ def test_get_feedback_list(self): assert feedback["contact_email"] == "colton.allen@sentry.io" # Test second item - feedback = get_response.data[1] + 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!" @@ -100,70 +115,29 @@ def test_get_feedback_list(self): 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}): - get_path = reverse(self.get_endpoint, args=[self.organization.slug]) - post_path = reverse(self.post_endpoint) - self.client.post( - post_path, - data=post_data_1, - **self.dsn_auth_headers, - ) - get_response = self.client.get(get_path, **self.dsn_auth_headers) + path = reverse(self.get_endpoint, args=[self.organization.slug]) + get_response = self.client.get(path) assert get_response.status_code == 404 - def test_no_authorization(self): - # No authorization should lead to unsuccessful GET - with self.feature({"organizations:user-feedback-ingest": False}): - get_path = reverse(self.get_endpoint, args=[self.organization.slug]) - post_path = reverse(self.post_endpoint) - self.client.post( - post_path, - data=post_data_1, - **self.dsn_auth_headers, - ) - get_response = self.client.get(get_path) - assert get_response.status_code == 401 - assert get_response.data == {"detail": "Authentication credentials were not provided."} - def test_bad_slug_path(self): # Bad slug in path should lead to unsuccessful GET with self.feature({"organizations:user-feedback-ingest": True}): - get_path = reverse(self.get_endpoint, args=["testslug123345"]) - post_path = reverse(self.post_endpoint) - self.client.post( - post_path, - data=post_data_1, - **self.dsn_auth_headers, - ) - get_response = self.client.get(get_path, **self.dsn_auth_headers) + 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") } - - def test_parameters(self): - # Testing GET parameters - # For now, only testing per_page; others (such as sort, query) are not well-defined or not necessary for MVP - with self.feature({"organizations:user-feedback-ingest": True}): - get_path = reverse(self.get_endpoint, args=[self.organization.slug]) - post_path = reverse(self.post_endpoint) - self.client.post( - post_path, - data=post_data_1, - **self.dsn_auth_headers, - ) - self.client.post( - post_path, - data=post_data_2, - **self.dsn_auth_headers, - ) - get_response = self.client.get( - path=get_path, - data={"per_page": 1}, - content_type="application/json", - **self.dsn_auth_headers, - ) - assert get_response.status_code == 200 - assert len(get_response.data) == 1 From d8dafc504b6d8d87bb1aca4c4e09e369b1148421 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 12 Sep 2023 17:05:07 -0700 Subject: [PATCH 07/11] fix typing --- src/sentry/feedback/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/feedback/serializers.py b/src/sentry/feedback/serializers.py index 42f848fc05ff70..43e85ed052bf19 100644 --- a/src/sentry/feedback/serializers.py +++ b/src/sentry/feedback/serializers.py @@ -24,7 +24,6 @@ class FeedbackResponseType(TypedDict): release: str status: str timestamp: str - url: str @register(Feedback) From 0a3c887b11a0463f615935c70c9551c6383e7e73 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 13 Sep 2023 08:28:52 -0700 Subject: [PATCH 08/11] update naming to feedback_id --- src/sentry/feedback/serializers.py | 4 ++-- tests/sentry/feedback/test_organization_feedback_index.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sentry/feedback/serializers.py b/src/sentry/feedback/serializers.py index 43e85ed052bf19..52e1b690122935 100644 --- a/src/sentry/feedback/serializers.py +++ b/src/sentry/feedback/serializers.py @@ -17,7 +17,7 @@ class FeedbackResponseType(TypedDict): sdk: Any contact_email: str environment: str - id: str + feedback_id: str message: str platform: str project_id: str @@ -41,7 +41,7 @@ def serialize(self, obj, attrs, user, **kwargs) -> FeedbackResponseType: "sdk": obj.data.get("sdk"), "contact_email": obj.data.get("feedback").get("contact_email"), "environment": obj.data.get("environment"), - "id": obj.feedback_id, + "feedback_id": obj.feedback_id, "message": obj.message, "platform": obj.data.get("platform"), "project_id": obj.project_id, diff --git a/tests/sentry/feedback/test_organization_feedback_index.py b/tests/sentry/feedback/test_organization_feedback_index.py index ac9ef1a71d4ec2..a4e7afbb3d5bfe 100644 --- a/tests/sentry/feedback/test_organization_feedback_index.py +++ b/tests/sentry/feedback/test_organization_feedback_index.py @@ -99,7 +99,7 @@ def test_get_feedback_list(self): 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["id"] == uuid.UUID("1ffe0775ac0f4417aed9de36d9f6f8dc") + assert feedback["feedback_id"] == uuid.UUID("1ffe0775ac0f4417aed9de36d9f6f8dc") assert feedback["platform"] == "javascript" assert feedback["sdk"]["name"] == "sentry.javascript.react" assert feedback["tags"]["key"] == "value" @@ -110,7 +110,7 @@ def test_get_feedback_list(self): 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["id"] == uuid.UUID("2ffe0775ac0f4417aed9de36d9f6f8ab") + 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" From a3bd505546701a4852c720206bb9bc3e13b2a9c3 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 13 Sep 2023 09:46:35 -0700 Subject: [PATCH 09/11] update snake case and remove unused code --- src/sentry/feedback/endpoints/feedback_ingest.py | 10 ++++++++-- .../feedback/endpoints/organization_feedback_index.py | 7 ------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/sentry/feedback/endpoints/feedback_ingest.py b/src/sentry/feedback/endpoints/feedback_ingest.py index 719857fd3e07d6..816a3a9aeeb510 100644 --- a/src/sentry/feedback/endpoints/feedback_ingest.py +++ b/src/sentry/feedback/endpoints/feedback_ingest.py @@ -43,8 +43,14 @@ class FeedbackValidator(serializers.Serializer): 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) + DeviceContext = serializers.JSONField( + required=False, + source="device_context", + ) + BrowserContext = serializers.JSONField( + required=False, + source="browser_context", + ) def validate(self, data): try: diff --git a/src/sentry/feedback/endpoints/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py index b172bd7689de68..b3005105d4ef29 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_index.py +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -8,7 +8,6 @@ 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.bases.project import ProjectPermission from sentry.api.paginator import OffsetPaginator from sentry.api.serializers.base import serialize from sentry.feedback.models import Feedback @@ -16,12 +15,6 @@ from sentry.models import Organization -class OrganizationFeedbackIndexPermission(ProjectPermission): - scope_map = { - "GET": ["project:read", "project:write", "project:admin"], - } - - @region_silo_endpoint class OrganizationFeedbackIndexEndpoint(OrganizationEndpoint): publish_status = { From 011a8f616f720468052f5e850732b3752d81ea27 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 13 Sep 2023 09:48:11 -0700 Subject: [PATCH 10/11] undo --- src/sentry/feedback/endpoints/feedback_ingest.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/sentry/feedback/endpoints/feedback_ingest.py b/src/sentry/feedback/endpoints/feedback_ingest.py index 816a3a9aeeb510..719857fd3e07d6 100644 --- a/src/sentry/feedback/endpoints/feedback_ingest.py +++ b/src/sentry/feedback/endpoints/feedback_ingest.py @@ -43,14 +43,8 @@ class FeedbackValidator(serializers.Serializer): tags = serializers.JSONField(required=False) user = serializers.JSONField(required=False) contexts = serializers.JSONField(required=False) - DeviceContext = serializers.JSONField( - required=False, - source="device_context", - ) - BrowserContext = serializers.JSONField( - required=False, - source="browser_context", - ) + BrowserContext = serializers.JSONField(required=False) + DeviceContext = serializers.JSONField(required=False) def validate(self, data): try: From 57228f7fab395000f8f8c0cd8b8247cc12bfda5b Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 13 Sep 2023 09:49:59 -0700 Subject: [PATCH 11/11] update filter to be by org id --- src/sentry/feedback/endpoints/organization_feedback_index.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/feedback/endpoints/organization_feedback_index.py b/src/sentry/feedback/endpoints/organization_feedback_index.py index b3005105d4ef29..1abb4a4ca587f4 100644 --- a/src/sentry/feedback/endpoints/organization_feedback_index.py +++ b/src/sentry/feedback/endpoints/organization_feedback_index.py @@ -26,7 +26,7 @@ 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(project_id__in=organization.project_set.all()) + feedback_list = Feedback.objects.filter(organization_id=organization.id) return self.paginate( request=request, queryset=feedback_list,