diff --git a/src/sentry/feedback/lib/types.py b/src/sentry/feedback/lib/types.py new file mode 100644 index 00000000000000..11d76d995fd6c6 --- /dev/null +++ b/src/sentry/feedback/lib/types.py @@ -0,0 +1,15 @@ +from typing import NotRequired, TypedDict + + +class UserReportDict(TypedDict): + """Use for weak type checking of user report data. Keys correspond to fields of the UserReport model.""" + + event_id: str + comments: str + # required for the model, but functions usually infer this from an explicit Project argument. + project_id: NotRequired[int] + name: NotRequired[str] + email: NotRequired[str] + environment_id: NotRequired[int] # defaults to "production". + group_id: NotRequired[int] + level: NotRequired[str] # defaults to "info". diff --git a/src/sentry/feedback/usecases/create_feedback.py b/src/sentry/feedback/usecases/create_feedback.py index 5844b3e64a1b53..806922a259e9bb 100644 --- a/src/sentry/feedback/usecases/create_feedback.py +++ b/src/sentry/feedback/usecases/create_feedback.py @@ -4,7 +4,7 @@ import random from datetime import UTC, datetime from enum import Enum -from typing import Any, TypedDict +from typing import Any from uuid import uuid4 import jsonschema @@ -12,6 +12,7 @@ from sentry import features, options from sentry.constants import DataCategory from sentry.eventstore.models import Event, GroupEvent +from sentry.feedback.lib.types import UserReportDict from sentry.feedback.usecases.spam_detection import is_spam, spam_detection_enabled from sentry.issues.grouptype import FeedbackGroup from sentry.issues.issue_occurrence import IssueEvidence, IssueOccurrence @@ -100,8 +101,10 @@ def fix_for_issue_platform(event_data: dict[str, Any]) -> dict[str, Any]: """ The issue platform has slightly different requirements than ingest for event schema, so we need to massage the data a bit. + * event["timestamp"] is converted to a UTC ISO string. * event["tags"] is coerced to a dict. - * If event["user"]["email"] is missing we try to set using the feedback context. + * If user or replay context is missing we try to set it using the feedback context. + * level defaults to "info" and environment defaults to "production". Returns: A dict[str, Any] conforming to sentry.issues.json_schemas.EVENT_PAYLOAD_SCHEMA. @@ -135,22 +138,19 @@ def fix_for_issue_platform(event_data: dict[str, Any]) -> dict[str, Any]: ret_event["request"] = event_data.get("request", {}) ret_event["user"] = event_data.get("user", {}) + if "name" in ret_event["user"]: + del ret_event["user"]["name"] - if event_data.get("dist") is not None: - del event_data["dist"] - if event_data.get("user", {}).get("name") is not None: - del event_data["user"]["name"] - if event_data.get("user", {}).get("isStaff") is not None: - del event_data["user"]["isStaff"] + if "isStaff" in ret_event["user"]: + del ret_event["user"]["isStaff"] - if event_data.get("user", {}).get("id") is not None: - event_data["user"]["id"] = str(event_data["user"]["id"]) + if "id" in ret_event["user"]: + ret_event["user"]["id"] = str(ret_event["user"]["id"]) # If no user email was provided specify the contact-email as the user-email. feedback_obj = event_data.get("contexts", {}).get("feedback", {}) - contact_email = feedback_obj.get("contact_email") - if not ret_event["user"].get("email", ""): - ret_event["user"]["email"] = contact_email + if "email" not in ret_event["user"]: + ret_event["user"]["email"] = feedback_obj.get("contact_email", "") # Force `tags` to be a dict if it's initially a list, # since we can't guarantee its type here. @@ -451,16 +451,8 @@ def auto_ignore_spam_feedbacks(project, issue_fingerprint): ########### -class UserReportShimDict(TypedDict): - name: str - email: str - comments: str - event_id: str - level: str - - def shim_to_feedback( - report: UserReportShimDict, + report: UserReportDict, event: Event | GroupEvent, project: Project, source: FeedbackCreationSource, @@ -491,7 +483,7 @@ def shim_to_feedback( "contexts": { "feedback": { "name": report.get("name", ""), - "contact_email": report["email"], + "contact_email": report.get("email", ""), "message": report["comments"], }, }, diff --git a/src/sentry/feedback/usecases/save_feedback_event.py b/src/sentry/feedback/usecases/save_feedback_event.py index b97468d97444a4..281bc7f051aeec 100644 --- a/src/sentry/feedback/usecases/save_feedback_event.py +++ b/src/sentry/feedback/usecases/save_feedback_event.py @@ -32,8 +32,15 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int): # Shim to UserReport feedback_context = fixed_event_data["contexts"]["feedback"] associated_event_id = feedback_context.get("associated_event_id") + if associated_event_id: project = Project.objects.get_from_cache(id=project_id) + timestamp = fixed_event_data["timestamp"] + start_time = ( + datetime.fromtimestamp(timestamp, tz=UTC) + if isinstance(timestamp, float) + else datetime.fromisoformat(timestamp) + ) save_userreport( project, { @@ -42,12 +49,12 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int): # XXX(aliu): including environment ensures the update_user_reports task # will not shim the report back to feedback. "environment_id": fixed_event_data.get("environment", "production"), - "name": feedback_context["name"], - "email": feedback_context["contact_email"], + "name": feedback_context.get("name", ""), + "email": feedback_context.get("contact_email", ""), "comments": feedback_context["message"], }, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE, - start_time=datetime.fromtimestamp(fixed_event_data["timestamp"], UTC), + start_time=start_time, ) metrics.incr("feedback.shim_to_userreport.success") diff --git a/src/sentry/ingest/userreport.py b/src/sentry/ingest/userreport.py index dbc3c10ad67aad..8f435b0983696c 100644 --- a/src/sentry/ingest/userreport.py +++ b/src/sentry/ingest/userreport.py @@ -10,6 +10,7 @@ from sentry import eventstore, options from sentry.constants import DataCategory from sentry.eventstore.models import Event, GroupEvent +from sentry.feedback.lib.types import UserReportDict from sentry.feedback.usecases.create_feedback import ( UNREAL_FEEDBACK_UNATTENDED_MESSAGE, FeedbackCreationSource, @@ -34,7 +35,7 @@ class Conflict(Exception): def save_userreport( project: Project, - report, + report: UserReportDict, source: FeedbackCreationSource, start_time: datetime | None = None, ) -> UserReport | None: @@ -100,7 +101,8 @@ def save_userreport( raise Conflict("Feedback for this event cannot be modified.") report["environment_id"] = event.get_environment().id - report["group_id"] = event.group_id + if event.group_id: + report["group_id"] = event.group_id # Save the report. try: @@ -126,7 +128,7 @@ def save_userreport( existing_report.update( name=report.get("name", ""), - email=report["email"], + email=report.get("email", ""), comments=report["comments"], ) report_instance = existing_report diff --git a/tests/sentry/api/endpoints/test_organization_user_reports.py b/tests/sentry/api/endpoints/test_organization_user_reports.py index b984e43cd7e307..10e46fe1351349 100644 --- a/tests/sentry/api/endpoints/test_organization_user_reports.py +++ b/tests/sentry/api/endpoints/test_organization_user_reports.py @@ -1,6 +1,7 @@ from datetime import UTC, datetime, timedelta from unittest.mock import patch +from sentry.feedback.lib.types import UserReportDict from sentry.feedback.usecases.create_feedback import FeedbackCreationSource from sentry.ingest.userreport import save_userreport from sentry.models.group import GroupStatus @@ -132,11 +133,12 @@ def test_with_event_user(self): ) # Simulate how ingest saves reports to get event_user connections - report_data = { + report_data: UserReportDict = { "event_id": event.event_id, "name": "", "email": "", "comments": "It broke", + "project_id": self.project_1.id, } save_userreport( self.project_1, report_data, FeedbackCreationSource.USER_REPORT_DJANGO_ENDPOINT diff --git a/tests/sentry/feedback/usecases/test_save_feedback_event.py b/tests/sentry/feedback/usecases/test_save_feedback_event.py index e2922bf384e8fd..83698fcc98ac7e 100644 --- a/tests/sentry/feedback/usecases/test_save_feedback_event.py +++ b/tests/sentry/feedback/usecases/test_save_feedback_event.py @@ -37,11 +37,19 @@ def test_save_feedback_event_no_associated_error( @django_db_all +@pytest.mark.parametrize( + "timestamp_format", + ["number", "iso"], +) def test_save_feedback_event_with_associated_error( - default_project, mock_create_feedback_issue, mock_save_userreport + default_project, mock_create_feedback_issue, mock_save_userreport, timestamp_format ): event_data = mock_feedback_event(default_project.id) event_data["contexts"]["feedback"]["associated_event_id"] = "abcd" * 8 + start_time = datetime.now(UTC) + event_data["timestamp"] = ( + start_time.timestamp() if timestamp_format == "number" else start_time.isoformat() + ) mock_create_feedback_issue.return_value = event_data save_feedback_event(event_data, default_project.id) @@ -62,5 +70,43 @@ def test_save_feedback_event_with_associated_error( "comments": feedback_context["message"], }, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE, - start_time=datetime.fromtimestamp(event_data["timestamp"], UTC), + start_time=start_time, + ) + + +@django_db_all +def test_save_feedback_event_with_associated_error_and_missing_fields( + default_project, mock_create_feedback_issue, mock_save_userreport +): + event_data = mock_feedback_event(default_project.id) + event_data["contexts"]["feedback"]["associated_event_id"] = "abcd" * 8 + start_time = datetime.now(UTC) + event_data["timestamp"] = start_time.isoformat() + + # Remove name, email, environment + del event_data["contexts"]["feedback"]["name"] + del event_data["contexts"]["feedback"]["contact_email"] + del event_data["environment"] + + mock_create_feedback_issue.return_value = event_data + + save_feedback_event(event_data, default_project.id) + + mock_create_feedback_issue.assert_called_once_with( + event_data, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE + ) + + feedback_context = event_data["contexts"]["feedback"] + mock_save_userreport.assert_called_once_with( + default_project, + { + "event_id": "abcd" * 8, + "project_id": default_project.id, + "environment_id": "production", + "name": "", + "email": "", + "comments": feedback_context["message"], + }, + FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE, + start_time=start_time, ) diff --git a/tests/sentry/ingest/test_userreport.py b/tests/sentry/ingest/test_userreport.py index 6654064842c16e..6b91182bb11a34 100644 --- a/tests/sentry/ingest/test_userreport.py +++ b/tests/sentry/ingest/test_userreport.py @@ -2,6 +2,7 @@ from django.utils import timezone +from sentry.feedback.lib.types import UserReportDict from sentry.feedback.usecases.create_feedback import ( UNREAL_FEEDBACK_UNATTENDED_MESSAGE, FeedbackCreationSource, @@ -73,7 +74,7 @@ def test_save_user_report_returns_instance(default_project, monkeypatch): ) # Test data - report = { + report: UserReportDict = { "event_id": "123456", "name": "Test User", "email": "test@example.com", @@ -91,7 +92,7 @@ def test_save_user_report_returns_instance(default_project, monkeypatch): @django_db_all def test_save_user_report_denylist(default_project, monkeypatch): monkeypatch.setattr("sentry.ingest.userreport.is_in_feedback_denylist", lambda org: True) - report = { + report: UserReportDict = { "event_id": "123456", "name": "Test User", "email": "test@example.com", @@ -116,7 +117,7 @@ def test_save_user_report_filters_large_message(default_project, monkeypatch): if not max_length: assert False, "Missing max_length for UserReport comments field!" - report = { + report: UserReportDict = { "event_id": "123456", "name": "Test User", "email": "test@example.com", @@ -145,7 +146,7 @@ def test_save_user_report_shims_if_event_found(default_project, monkeypatch): mock_shim_to_feedback = Mock() monkeypatch.setattr("sentry.ingest.userreport.shim_to_feedback", mock_shim_to_feedback) - report = { + report: UserReportDict = { "event_id": event.event_id, "name": "Test User", "email": "test@example.com", @@ -176,7 +177,7 @@ def test_save_user_report_does_not_shim_if_event_found_but_source_is_new_feedbac mock_shim_to_feedback = Mock() monkeypatch.setattr("sentry.ingest.userreport.shim_to_feedback", mock_shim_to_feedback) - report = { + report: UserReportDict = { "event_id": event.event_id, "name": "Test User", "email": "test@example.com",