Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feedback): add organization feedback index GET endpoint #56065

Merged
merged 20 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
782dd26
fix(replays api): change limit to be per_page
michellewzhang Sep 6, 2023
f8b0a6f
Revert "fix(replays api): change limit to be per_page"
michellewzhang Sep 6, 2023
6658164
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 7, 2023
c0a34c8
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 7, 2023
457e1fb
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 7, 2023
cdf3c8a
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 7, 2023
e829175
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 8, 2023
be1bf62
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 8, 2023
5996201
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 9, 2023
5705dc1
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 11, 2023
33abf63
Merge branch 'master' of github.com:getsentry/sentry
michellewzhang Sep 12, 2023
8aaaf4f
feat(feedback): add organization feedback index GET endpoint
michellewzhang Sep 12, 2023
76c019d
add tests
michellewzhang Sep 12, 2023
02444cc
update model with organization_id stored and add per_page for endpoint
michellewzhang Sep 12, 2023
8ffe896
update endpoint to inherit from OrganizationEndpoint
michellewzhang Sep 12, 2023
d8dafc5
fix typing
michellewzhang Sep 13, 2023
0a3c887
update naming to feedback_id
michellewzhang Sep 13, 2023
a3bd505
update snake case and remove unused code
michellewzhang Sep 13, 2023
011a8f6
undo
michellewzhang Sep 13, 2023
57228f7
update filter to be by org id
michellewzhang Sep 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/sentry/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -1746,6 +1747,12 @@
OrganizationTransactionAnomalyDetectionEndpoint.as_view(),
name="sentry-api-0-organization-transaction-anomaly-detection",
),
# Feedback
re_path(
r"^(?P<organization_slug>[^\/]+)/feedback/$",
OrganizationFeedbackIndexEndpoint.as_view(),
name="sentry-api-0-organization-feedback-index",
),
# relay usage
re_path(
r"^(?P<organization_slug>[^\/]+)/relay_usage/$",
Expand Down
18 changes: 16 additions & 2 deletions src/sentry/feedback/endpoints/feedback_ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ class FeedbackValidator(serializers.Serializer):
request = serializers.JSONField(required=False)
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",
)

def validate(self, data):
try:
Expand All @@ -56,14 +65,17 @@ 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"),
Comment on lines +63 to +64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add a shim in future PR for these fields

}
ret["date_added"] = datetime.datetime.fromtimestamp(data["timestamp"])
ret["feedback_id"] = data.get("event_id") or uuid4().hex
ret["url"] = data["feedback"]["url"]
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")
Expand Down Expand Up @@ -144,7 +156,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)

Expand Down
35 changes: 35 additions & 0 deletions src/sentry/feedback/endpoints/organization_feedback_index.py
Original file line number Diff line number Diff line change
@@ -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(project_id__in=organization.project_set.all())
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved
return self.paginate(
request=request,
queryset=feedback_list,
on_results=lambda x: serialize(x, request.user, FeedbackSerializer()),
paginator_cls=OffsetPaginator,
)
8 changes: 6 additions & 2 deletions src/sentry/feedback/migrations/0001_feedback.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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={
Expand Down
5 changes: 3 additions & 2 deletions src/sentry/feedback/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
53 changes: 53 additions & 0 deletions src/sentry/feedback/serializers.py
Original file line number Diff line number Diff line change
@@ -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
10 changes: 8 additions & 2 deletions tests/sentry/feedback/test_feedback_ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
}


Expand Down Expand Up @@ -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
143 changes: 143 additions & 0 deletions tests/sentry/feedback/test_organization_feedback_index.py
Original file line number Diff line number Diff line change
@@ -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")
}
Loading