-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(releases): Add all/new events components for org releases #11369
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
Conversation
996b093 to
9864ca4
Compare
| <Link | ||
| to={{ | ||
| pathname: `/organizations/${orgId}/issues/`, | ||
| query: {query: 'release:' + this.context.release.version}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is version not wrapped in " here? Should you URLencode release.version? It can contain URL unsafe data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using query seems to automatically encode this for us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not exactly sure what the impact of wrapping in " is in the stream view (afaik both are valid) but since i'm not confident I've gone with the same implementation as the project releases version here https://github.com/getsentry/sentry/blob/master/src/sentry/static/sentry/app/views/releases/detail/project/releaseAllEvents.jsx#L27
billyvg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two components are pretty similar, they could be the same component with 2 additional props - it's fine as is though.
src/sentry/static/sentry/app/views/releases/detail/organization/releaseAllEvents.jsx
Outdated
Show resolved
Hide resolved
src/sentry/static/sentry/app/views/releases/detail/organization/releaseAllEvents.jsx
Outdated
Show resolved
Hide resolved
src/sentry/static/sentry/app/views/releases/detail/organization/releaseNewEvents.jsx
Outdated
Show resolved
Hide resolved
| query: {query: 'first-release:' + this.context.release.version}, | ||
| }} | ||
| > | ||
| <span className="icon icon-open" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an svg icon available?
No description provided.