Skip to content

Commit cdf6162

Browse files
authored
fix(feedback): fix environment id for shim to userreport (#89985)
Fixes SENTRY-3SFQ Followup to #89685 Improves the test coverage so we assert a UR is actually saved, instead of asserting the arguments to `save_userreport`.
1 parent 778e52d commit cdf6162

File tree

2 files changed

+85
-44
lines changed

2 files changed

+85
-44
lines changed

src/sentry/feedback/usecases/save_feedback_event.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from sentry.feedback.usecases.create_feedback import FeedbackCreationSource, create_feedback_issue
77
from sentry.ingest.userreport import save_userreport
8+
from sentry.models.environment import Environment
89
from sentry.models.project import Project
910
from sentry.utils import metrics
1011

@@ -35,6 +36,10 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int):
3536

3637
if associated_event_id:
3738
project = Project.objects.get_from_cache(id=project_id)
39+
environment = Environment.objects.get(
40+
organization_id=project.organization_id,
41+
name=fixed_event_data.get("environment", "production"),
42+
)
3843
timestamp = fixed_event_data["timestamp"]
3944
start_time = (
4045
datetime.fromtimestamp(timestamp, tz=UTC)
@@ -48,7 +53,7 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int):
4853
"project_id": project_id,
4954
# XXX(aliu): including environment ensures the update_user_reports task
5055
# will not shim the report back to feedback.
51-
"environment_id": fixed_event_data.get("environment", "production"),
56+
"environment_id": environment.id,
5257
"name": feedback_context.get("name", ""),
5358
"email": feedback_context.get("contact_email", ""),
5459
"comments": feedback_context["message"],

tests/sentry/feedback/usecases/test_save_feedback_event.py

Lines changed: 79 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33

44
import pytest
55

6+
from sentry.eventstore.models import Event
67
from sentry.feedback.usecases.create_feedback import FeedbackCreationSource
78
from sentry.feedback.usecases.save_feedback_event import save_feedback_event
9+
from sentry.models.environment import Environment
10+
from sentry.models.userreport import UserReport
11+
from sentry.testutils.factories import Factories
812
from sentry.testutils.pytest.fixtures import django_db_all
913
from tests.sentry.feedback.usecases.test_create_feedback import mock_feedback_event
1014

@@ -15,16 +19,18 @@ def mock_create_feedback_issue():
1519
yield m
1620

1721

18-
@pytest.fixture
19-
def mock_save_userreport():
20-
with mock.patch("sentry.feedback.usecases.save_feedback_event.save_userreport") as m:
21-
yield m
22+
def create_test_event(project_id: int, environment: Environment) -> Event:
23+
return Factories.store_event(
24+
{
25+
"event_id": "ff1c2e3d4b5a6978899abbccddeeff00",
26+
"environment": environment.name,
27+
},
28+
project_id,
29+
)
2230

2331

2432
@django_db_all
25-
def test_save_feedback_event_no_associated_error(
26-
default_project, mock_create_feedback_issue, mock_save_userreport
27-
):
33+
def test_save_feedback_event_no_associated_event(default_project, mock_create_feedback_issue):
2834
event_data = mock_feedback_event(default_project.id)
2935
mock_create_feedback_issue.return_value = None
3036

@@ -33,23 +39,28 @@ def test_save_feedback_event_no_associated_error(
3339
mock_create_feedback_issue.assert_called_once_with(
3440
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
3541
)
36-
mock_save_userreport.assert_not_called()
42+
assert UserReport.objects.count() == 0
3743

3844

3945
@django_db_all
4046
@pytest.mark.parametrize(
4147
"timestamp_format",
4248
["number", "iso"],
4349
)
44-
def test_save_feedback_event_with_associated_error(
45-
default_project, mock_create_feedback_issue, mock_save_userreport, timestamp_format
50+
def test_save_feedback_event_with_associated_event(
51+
default_project, mock_create_feedback_issue, timestamp_format
4652
):
53+
environment = Factories.create_environment(default_project, name="production")
54+
assoc_event = create_test_event(default_project.id, environment)
55+
4756
event_data = mock_feedback_event(default_project.id)
48-
event_data["contexts"]["feedback"]["associated_event_id"] = "abcd" * 8
49-
start_time = datetime.now(UTC)
57+
event_data["contexts"]["feedback"]["associated_event_id"] = assoc_event.event_id
5058
event_data["timestamp"] = (
51-
start_time.timestamp() if timestamp_format == "number" else start_time.isoformat()
59+
datetime.now(UTC).timestamp()
60+
if timestamp_format == "number"
61+
else datetime.now(UTC).isoformat()
5262
)
63+
event_data["environment"] = "production"
5364
mock_create_feedback_issue.return_value = event_data
5465

5566
save_feedback_event(event_data, default_project.id)
@@ -58,30 +69,59 @@ def test_save_feedback_event_with_associated_error(
5869
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
5970
)
6071

72+
assert UserReport.objects.count() == 1
73+
report = UserReport.objects.get()
74+
6175
feedback_context = event_data["contexts"]["feedback"]
62-
mock_save_userreport.assert_called_once_with(
63-
default_project,
64-
{
65-
"event_id": "abcd" * 8,
66-
"project_id": default_project.id,
67-
"environment_id": event_data["environment"],
68-
"name": feedback_context["name"],
69-
"email": feedback_context["contact_email"],
70-
"comments": feedback_context["message"],
71-
},
72-
FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE,
73-
start_time=start_time,
74-
)
76+
assert report.event_id == assoc_event.event_id
77+
assert report.project_id == default_project.id
78+
assert report.group_id == assoc_event.group_id
79+
assert report.environment_id == environment.id
80+
assert report.name == feedback_context["name"]
81+
assert report.email == feedback_context["contact_email"]
82+
assert report.comments == feedback_context["message"]
7583

7684

7785
@django_db_all
78-
def test_save_feedback_event_with_associated_error_and_missing_fields(
79-
default_project, mock_create_feedback_issue, mock_save_userreport
86+
def test_save_feedback_event_with_unprocessed_associated_event(
87+
default_project,
88+
mock_create_feedback_issue,
8089
):
90+
environment = Factories.create_environment(default_project, name="production")
91+
8192
event_data = mock_feedback_event(default_project.id)
8293
event_data["contexts"]["feedback"]["associated_event_id"] = "abcd" * 8
83-
start_time = datetime.now(UTC)
84-
event_data["timestamp"] = start_time.isoformat()
94+
event_data["timestamp"] = datetime.now(UTC).isoformat()
95+
event_data["environment"] = "production"
96+
mock_create_feedback_issue.return_value = event_data
97+
98+
save_feedback_event(event_data, default_project.id)
99+
100+
mock_create_feedback_issue.assert_called_once_with(
101+
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
102+
)
103+
104+
# UserReport is still created, with null group_id.
105+
assert UserReport.objects.count() == 1
106+
report = UserReport.objects.get()
107+
108+
feedback_context = event_data["contexts"]["feedback"]
109+
assert report.event_id == "abcd" * 8
110+
assert report.project_id == default_project.id
111+
assert report.group_id is None
112+
assert report.environment_id == environment.id
113+
assert report.name == feedback_context["name"]
114+
assert report.email == feedback_context["contact_email"]
115+
assert report.comments == feedback_context["message"]
116+
117+
118+
@django_db_all
119+
def test_save_feedback_event_with_missing_fields(default_project, mock_create_feedback_issue):
120+
environment = Factories.create_environment(default_project, name="production")
121+
122+
event_data = mock_feedback_event(default_project.id)
123+
event_data["contexts"]["feedback"]["associated_event_id"] = "abcd" * 8
124+
event_data["timestamp"] = datetime.now(UTC).isoformat()
85125

86126
# Remove name, email, environment
87127
del event_data["contexts"]["feedback"]["name"]
@@ -96,17 +136,13 @@ def test_save_feedback_event_with_associated_error_and_missing_fields(
96136
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
97137
)
98138

139+
assert UserReport.objects.count() == 1
140+
report = UserReport.objects.get()
141+
99142
feedback_context = event_data["contexts"]["feedback"]
100-
mock_save_userreport.assert_called_once_with(
101-
default_project,
102-
{
103-
"event_id": "abcd" * 8,
104-
"project_id": default_project.id,
105-
"environment_id": "production",
106-
"name": "",
107-
"email": "",
108-
"comments": feedback_context["message"],
109-
},
110-
FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE,
111-
start_time=start_time,
112-
)
143+
assert report.event_id == "abcd" * 8
144+
assert report.project_id == default_project.id
145+
assert report.environment_id == environment.id
146+
assert report.name == ""
147+
assert report.email == ""
148+
assert report.comments == feedback_context["message"]

0 commit comments

Comments
 (0)