Skip to content

feat(feedback): set feedback issue to ignored if spam #69479

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

Merged
merged 2 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 14 additions & 3 deletions src/sentry/feedback/usecases/create_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from sentry.issues.issue_occurrence import IssueEvidence, IssueOccurrence
from sentry.issues.json_schemas import EVENT_PAYLOAD_SCHEMA, LEGACY_EVENT_PAYLOAD_SCHEMA
from sentry.issues.producer import PayloadType, produce_occurrence_to_kafka
from sentry.issues.status_change_message import StatusChangeMessage
from sentry.models.group import GroupStatus
from sentry.models.project import Project
from sentry.signals import first_feedback_received, first_new_feedback_received
from sentry.utils import metrics
Expand Down Expand Up @@ -194,13 +196,12 @@ def create_feedback_issue(event, project_id, source: FeedbackCreationSource):
evidence_data, evidence_display = make_evidence(
event["contexts"]["feedback"], source, is_message_spam
)
issue_fingerprint = [uuid4().hex]
occurrence = IssueOccurrence(
id=uuid4().hex,
event_id=event.get("event_id") or uuid4().hex,
project_id=project_id,
fingerprint=[
uuid4().hex
], # random UUID for fingerprint so feedbacks are grouped individually
fingerprint=issue_fingerprint, # random UUID for fingerprint so feedbacks are grouped individually
issue_title="User Feedback",
subtitle=event["contexts"]["feedback"]["message"],
resource_id=None,
Expand Down Expand Up @@ -240,6 +241,16 @@ def create_feedback_issue(event, project_id, source: FeedbackCreationSource):
produce_occurrence_to_kafka(
payload_type=PayloadType.OCCURRENCE, occurrence=occurrence, event_data=event_fixed
)
if is_message_spam:
produce_occurrence_to_kafka(
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is the bug you're referring to but is it possible for this message to be processed before the PayloadType.OCCURRENCE message? would the group not exist in that case?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that's the bug i'm referring to, and that's what steve is fixing here: #69466

payload_type=PayloadType.STATUS_CHANGE,
status_change=StatusChangeMessage(
fingerprint=issue_fingerprint,
project_id=project_id,
new_status=GroupStatus.RESOLVED,
new_substatus=None,
),
)
metrics.incr(
"feedback.create_feedback_issue.produced_occurrence",
tags={"referrer": source.value},
Expand Down
17 changes: 14 additions & 3 deletions tests/sentry/feedback/usecases/test_create_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
fix_for_issue_platform,
validate_issue_platform_event_schema,
)
from sentry.models.group import GroupStatus
from sentry.testutils.helpers import Feature
from sentry.testutils.pytest.fixtures import django_db_all

Expand Down Expand Up @@ -528,12 +529,22 @@ def dummy_response(*args, **kwargs):
# Check if the 'is_spam' evidence in the Kafka message matches the expected result
is_spam_evidence = [
evidence.value
for evidence in mock_produce_occurrence_to_kafka.call_args.kwargs[
"occurrence"
].evidence_display
for evidence in mock_produce_occurrence_to_kafka.call_args_list[0]
.kwargs["occurrence"]
.evidence_display
if evidence.name == "is_spam"
]
found_is_spam = is_spam_evidence[0] if is_spam_evidence else None
assert (
found_is_spam == expected_result
), f"Expected {expected_result} but found {found_is_spam} for {input_message} and feature flag {feature_flag}"

if expected_result and feature_flag:
assert (
mock_produce_occurrence_to_kafka.call_args_list[1]
.kwargs["status_change"]
.new_status
== GroupStatus.RESOLVED
)
if not (expected_result and feature_flag):
assert mock_produce_occurrence_to_kafka.call_count == 1
Loading