Skip to content
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

fix(ui): Use the correct docs link for user feedback docs #23585

Merged
merged 6 commits into from
Feb 18, 2021

Conversation

BYK
Copy link
Member

@BYK BYK commented Feb 3, 2021

Turns out there was an information architecture change over at getsentry/sentry-docs recently, requiring a platform name for the docs of user feedback. This patch tries to fix that. Fixes #23551.

Turns out there was an information architecture change over at getsentry/sentry-docs recently, requiring a platform name for the docs of user feedback. This patch tries to fix that. Fixes #23551.
@@ -108,7 +108,9 @@ class UserFeedbackEmpty extends React.Component<Props> {
eventName: 'User Feedback Docs Clicked',
})
}
href="https://docs.sentry.io/enriching-error-data/user-feedback/"
href={`https://docs.sentry.io/platforms/${
this.props.projects[0].platform || 'javascript'
Copy link
Member

Choose a reason for hiding this comment

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

In case there are no projects

Suggested change
this.props.projects[0].platform || 'javascript'
this.props.projects?.[0].platform || 'javascript'

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we can reach this line if projects is an empty array due to the check above. Am I missing something?

@BYK
Copy link
Member Author

BYK commented Feb 3, 2021

Verified the fix with the Vercel preview, it works. Added a small fix regarding selected projects on user feedback view.

Also /cc @PeloWriter & @dcramer to prevent similar things in the future. We may want to set up alerts on number of 404s on the docs.

@BYK BYK marked this pull request as ready for review February 3, 2021 19:06
@BYK BYK enabled auto-merge (squash) February 3, 2021 19:06
@BYK BYK requested a review from billyvg February 3, 2021 19:06
@billyvg
Copy link
Member

billyvg commented Feb 3, 2021

Doesn't seem like platforms always matches up to docs? e.g. https://docs.sentry.io/platforms/javascript-react/enriching-events/user-feedback/

@BYK
Copy link
Member Author

BYK commented Feb 3, 2021

Doesn't seem like platforms always matches up to docs? e.g. docs.sentry.io/platforms/javascript-react/enriching-events/user-feedback

Ugh, good catch. I need the parent platform id. I'll see if this is easy to get, otherwise I just need good ol' platform.split('-', 1)[0].

@BYK
Copy link
Member Author

BYK commented Feb 3, 2021

@billyvg ugh, so not only my hacky split() solution wouldn't work due to things like react-native as a platform name, we don't have the docs for user feedback for all platforms.

Here's what I think:

  1. This patch definitely makes things less broken so we should ship it mostly as it is, unless you have huge objections
  2. We should figure out a way to detect on which platforms we support user-feedback and for the ones we don't, direct people to the raw API docs over at https://docs.sentry.io/api/projects/submit-user-feedback/

@PeloWriter
Copy link

PeloWriter commented Feb 3, 2021

Can we extrapolate support? We list the platforms that don't support User Feedback in the yaml frontmatter of src/platforms/common/enriching-events/user-feedback.mdx

@PeloWriter
Copy link

It would be good to find a more automated fix. We've also got breakage for the link in Settings > Tags "add custom tags"
Screen Shot 2021-02-03 at 1 11 31 PM

@billyvg
Copy link
Member

billyvg commented Feb 10, 2021

Going to remove myself from reviewers, feel free to re-tag me if there are updates

@BYK
Copy link
Member Author

BYK commented Feb 11, 2021

@evanpurkhiser since you did #23720 recently, I need your eyes on this.

@evanpurkhiser
Copy link
Member

Should we just drop trying to incorporate the platform key?

@BYK
Copy link
Member Author

BYK commented Feb 11, 2021

Should we just drop trying to incorporate the platform key?

I mean, wouldn't it be nice to take them directly where they should be going instead of introducing another hop? I think the missing link here is, if the platform does not support this, it should take you to the platform selection page instead of giving you a 404. Or we should fetch that information and not display this link/page at all (see getsentry/sentry-release-registry#39 for that part)

@BYK BYK disabled auto-merge February 11, 2021 19:47
@BYK BYK merged commit 39891bb into master Feb 18, 2021
@BYK BYK deleted the byk/fix/userFeedback-docs-link branch February 18, 2021 19:48
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

404 when attempting to 'Read the Docs' in User Feedback
4 participants