Skip to content

Commit 6eb2260

Browse files
committed
save_event_feedback renaming and commenting for upcoming pr
1 parent 24e5e4d commit 6eb2260

File tree

3 files changed

+19
-16
lines changed

3 files changed

+19
-16
lines changed

src/sentry/feedback/usecases/save_feedback_event.py renamed to src/sentry/feedback/usecases/save_event_feedback.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313
logger = logging.getLogger(__name__)
1414

1515

16-
def save_feedback_event(event_data: Mapping[str, Any], project_id: int):
17-
"""Saves a feedback from a feedback event envelope.
16+
def save_event_feedback(event_data: Mapping[str, Any], project_id: int):
17+
"""Saves feedback given data in an event format. This function should only be called by the new feedback consumer's ingest strategy, to process feedback envelopes (feedback v2).
18+
It is currently instrumented as a task in sentry.tasks.store.
1819
1920
If the save is successful and the `associated_event_id` field is present, this will
20-
also save a UserReport in Postgres. This is to ensure the feedback can be queried by
21-
group_id, which is hard to associate in clickhouse.
21+
also save a UserReport in Postgres (shim to v1). This is to allow queries by the group_id relation, which we don't have in clickhouse.
2222
"""
2323
if not isinstance(event_data, dict):
2424
event_data = dict(event_data)
@@ -32,6 +32,7 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int):
3232

3333
try:
3434
# Shim to UserReport
35+
# TODO: this logic should be extracted to a shim_to_userreport function which returns a report dict. After that this function can be removed and the store task can directly call feedback ingest functions.
3536
feedback_context = fixed_event_data["contexts"]["feedback"]
3637
associated_event_id = feedback_context.get("associated_event_id")
3738

src/sentry/tasks/store.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
from sentry.constants import DEFAULT_STORE_NORMALIZER_ARGS
1717
from sentry.datascrubbing import scrub_data
1818
from sentry.eventstore import processing
19-
from sentry.feedback.usecases.save_feedback_event import save_feedback_event
19+
from sentry.feedback.usecases.save_event_feedback import (
20+
save_event_feedback as save_event_feedback_impl,
21+
)
2022
from sentry.ingest.types import ConsumerType
2123
from sentry.killswitches import killswitch_matches_context
2224
from sentry.lang.native.symbolicator import SymbolicatorTaskKind
@@ -703,7 +705,7 @@ def save_event_feedback(
703705
project_id: int,
704706
**kwargs: Any,
705707
) -> None:
706-
save_feedback_event(data, project_id)
708+
save_event_feedback_impl(data, project_id)
707709

708710

709711
@instrumented_task(

tests/sentry/feedback/test_save_feedback_event.py renamed to tests/sentry/feedback/test_save_event_feedback.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from sentry.eventstore.models import Event
77
from sentry.feedback.lib.utils import FeedbackCreationSource
8-
from sentry.feedback.usecases.save_feedback_event import save_feedback_event
8+
from sentry.feedback.usecases.save_event_feedback import save_event_feedback
99
from sentry.models.environment import Environment
1010
from sentry.models.userreport import UserReport
1111
from sentry.testutils.factories import Factories
@@ -15,7 +15,7 @@
1515

1616
@pytest.fixture
1717
def mock_create_feedback_issue():
18-
with mock.patch("sentry.feedback.usecases.save_feedback_event.create_feedback_issue") as m:
18+
with mock.patch("sentry.feedback.usecases.save_event_feedback.create_feedback_issue") as m:
1919
yield m
2020

2121

@@ -30,11 +30,11 @@ def create_test_event(project_id: int, environment: Environment) -> Event:
3030

3131

3232
@django_db_all
33-
def test_save_feedback_event_no_associated_event(default_project, mock_create_feedback_issue):
33+
def test_save_event_feedback_no_associated_event(default_project, mock_create_feedback_issue):
3434
event_data = mock_feedback_event(default_project.id)
3535
mock_create_feedback_issue.return_value = None
3636

37-
save_feedback_event(event_data, default_project.id)
37+
save_event_feedback(event_data, default_project.id)
3838

3939
mock_create_feedback_issue.assert_called_once_with(
4040
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
@@ -47,7 +47,7 @@ def test_save_feedback_event_no_associated_event(default_project, mock_create_fe
4747
"timestamp_format",
4848
["number", "iso"],
4949
)
50-
def test_save_feedback_event_with_associated_event(
50+
def test_save_event_feedback_with_associated_event(
5151
default_project, mock_create_feedback_issue, timestamp_format
5252
):
5353
environment = Factories.create_environment(default_project, name="production")
@@ -63,7 +63,7 @@ def test_save_feedback_event_with_associated_event(
6363
event_data["environment"] = "production"
6464
mock_create_feedback_issue.return_value = event_data
6565

66-
save_feedback_event(event_data, default_project.id)
66+
save_event_feedback(event_data, default_project.id)
6767

6868
mock_create_feedback_issue.assert_called_once_with(
6969
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
@@ -83,7 +83,7 @@ def test_save_feedback_event_with_associated_event(
8383

8484

8585
@django_db_all
86-
def test_save_feedback_event_with_unprocessed_associated_event(
86+
def test_save_event_feedback_with_unprocessed_associated_event(
8787
default_project,
8888
mock_create_feedback_issue,
8989
):
@@ -95,7 +95,7 @@ def test_save_feedback_event_with_unprocessed_associated_event(
9595
event_data["environment"] = "production"
9696
mock_create_feedback_issue.return_value = event_data
9797

98-
save_feedback_event(event_data, default_project.id)
98+
save_event_feedback(event_data, default_project.id)
9999

100100
mock_create_feedback_issue.assert_called_once_with(
101101
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
@@ -116,7 +116,7 @@ def test_save_feedback_event_with_unprocessed_associated_event(
116116

117117

118118
@django_db_all
119-
def test_save_feedback_event_with_missing_fields(default_project, mock_create_feedback_issue):
119+
def test_save_event_feedback_with_missing_fields(default_project, mock_create_feedback_issue):
120120
environment = Factories.create_environment(default_project, name="production")
121121

122122
event_data = mock_feedback_event(default_project.id)
@@ -130,7 +130,7 @@ def test_save_feedback_event_with_missing_fields(default_project, mock_create_fe
130130

131131
mock_create_feedback_issue.return_value = event_data
132132

133-
save_feedback_event(event_data, default_project.id)
133+
save_event_feedback(event_data, default_project.id)
134134

135135
mock_create_feedback_issue.assert_called_once_with(
136136
event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE

0 commit comments

Comments
 (0)