Skip to content

ref(feedback): display crash report icon for django endpoint feedbacks#90133

Merged
aliu39 merged 4 commits intomasterfrom
aliu/crash-report-icon
May 6, 2025
Merged

ref(feedback): display crash report icon for django endpoint feedbacks#90133
aliu39 merged 4 commits intomasterfrom
aliu/crash-report-icon

Conversation

@aliu39
Copy link
Copy Markdown
Member

@aliu39 aliu39 commented Apr 23, 2025

Missed a source here for displaying the red 💀 symbol in feedback list. This source is still used, w/5.7k last month

@aliu39 aliu39 requested review from a team as code owners April 23, 2025 04:15
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 23, 2025
Comment thread static/app/utils/feedback/types.tsx Outdated
Comment on lines +54 to +58
export const CRASH_REPORT_SOURCES: string[] = [
'user_report_envelope',
'user_report_sentry_django_endpoint',
'crash_report_embed_form',
] as const;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a bit unusual to have non-types in a types.tsx, but it's not that much different if this were an enum

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the file could be renamed to utils.tsx but that might be worse as it's already in the utils folder 💀

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

types.tsx & utils.tsx & consts.tsx & index.tsx are all just bad names in general... grab bags of stuff :(

hasLinkedError.tsx could be a good name for something like this:

const CRASH_REPORT_SOURCES: string[] = [
  'user_report_envelope',
  'user_report_sentry_django_endpoint',
  'crash_report_embed_form',
] as const;

export default function hasLinkedError(item: FeedbackIssueListItem): boolean {
  return CRASH_REPORT_SOURCES.includes(
    item.metadata.source ?? ''
  );
}

but it's a drop in the ocean to change things here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I like the hasLinkedError.tsx file idea!

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 6, 2025
@github-actions

This comment was marked as outdated.

@aliu39 aliu39 enabled auto-merge (squash) May 6, 2025 18:45
@aliu39
Copy link
Copy Markdown
Member Author

aliu39 commented May 6, 2025

I realized with the capability of linking errors in the new feedback API, this icon won't show up for that source. The most accurate fix is to render it conditionally when associated_event_id exists, but we need a way to store this in FeedbackIssueListItem metadata first

@aliu39 aliu39 merged commit 5237b9b into master May 6, 2025
41 checks passed
@aliu39 aliu39 deleted the aliu/crash-report-icon branch May 6, 2025 18:58
ryan953 added a commit to getsentry/sentry-toolbar that referenced this pull request May 6, 2025
andrewshie-sentry pushed a commit that referenced this pull request May 12, 2025
#90133)

Missed a source here for displaying the red 💀 symbol in feedback list.
This source is still used, w/5.7k last month
@github-actions github-actions Bot locked and limited conversation to collaborators May 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants