Skip to content

Conversation

@adhiraj
Copy link
Contributor

@adhiraj adhiraj commented Apr 7, 2019

Seems to have broken in #11422 with Sentry 10 changes.

Tried putting it in various places like componentDidMount or onGroupChange, but they either don't have the data yet, or are called multiple times. Putting it right after the API call seems to come closest to logging it once.

Seems to have broken in #11422 with Sentry 10 changes.
@adhiraj adhiraj requested a review from lynnagara April 7, 2019 00:17
@adhiraj
Copy link
Contributor Author

adhiraj commented Apr 7, 2019

@lynnagara where is https://github.com/getsentry/sentry/blob/master/src/sentry/static/sentry/app/views/groupDetails/project/index.jsx#L38 being used? there still seem to be some calls to that component, though much fewer than historically

@lynnagara
Copy link
Member

It's a lot less than historically since it's currently only tracked for users in the Sentry 9 flow.

For users in Sentry 10, it might be cleaner to add the analytics in this file instead - https://github.com/getsentry/sentry/blob/master/src/sentry/static/sentry/app/views/groupDetails/organization/index.jsx. The org and group ID should already be there when this component is mounted. Putting it in the success callback here will result it being called too many times when you toggle environment, etc.

@lynnagara
Copy link
Member

Ah, unfortunately we remove project from all of the URLs so it's not as easy as before

@lynnagara
Copy link
Member

nit: When you do squash and merge, can you make the commit prefix fix: vs (fix), ref: https://docs.sentry.io/development/contribute/contributing/#commit-guidelines

@adhiraj adhiraj merged commit 290b518 into master Apr 10, 2019
@adhiraj adhiraj deleted the fix_analytics branch April 10, 2019 18:23
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
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.

3 participants