-
Notifications
You must be signed in to change notification settings - Fork 492
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
6938 analytics new buttons #7008
Conversation
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.
Looks good to me. I checked the selectors on v4.20 with the normal download, download with the dropdown choices for .tab files, and previewers, on dataset and file pages. The only note I have is that the 'category' reported to Google for the download for Tab files is the label for the specific option rather than the generic 'Download' - probably useful to have the added info (I mention this partly because when I tested I accidentally put a download category filter on and wondered where the events were.)
@kcondon wanted to outline some code clean up in my last commit here.
That last bullet is the major piece. In the original PR to change large bulk downloads from GET's to POST's, the javascript snippet that was added to the file-download-button-fragment.xhtml resulted in the same javascript snippet being added multiple times to the source code for every file table row, while it only needs to be added to the page once. I moved that snippet to the table header code in filesFragment, so it is grouped under the Download button that requires it, and it only displays in the source code once. Also followed up with @qqmyers to confirm that the behavior I was seeing in my browser console was correct for a bulk download of 1,500 files. (See screenshot.) |
@qqmyers, after looking at my test Google Analytics dashboard, I am leaning towards useful. Previously the only download click events tracked for the single-click, non-tabular files, had "Download" for both Event Category and Event Action. Providing this additional information in the category still allows the "Download" totals to be filtered in reports under Event Action. That said, I am happy to learn more about how these events and reports are used for your installations, and if there are further improvements that can be made. We are just starting to think about how we drill into them ourselves, as we've only collected a couple months of analytics from our installation. |
@qqmyers gave a good tip for testing: check the comments in the analytics code for which to btns to check:
|
@mheppler All of the above works except the share button, which is not yet hooked up. |
What this PR does / why we need it:
With the new button UI delivered in #6684, the analytics functionality was reviewed. This fixes some issues as a result of the new UI, as well as some bugs that were previously not reported. Updates to the sample code provided in the Installation Guide.
Which issue(s) this PR closes:
Closes #6938 Analytics - Event tracking code not capture all the btn clicks
Special notes for your reviewer:
These fixes should work for both 4.20 and 5.0 UI's.
Suggestions on how to test this:
Because this analytics code is used by installations that run 4.20, it should be tested on both the new 5.0 btn UI, as well as the previous 4.20 UI.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
N/A
Is there a release notes update needed for this change?:
Yes, sysadmin who have < 4.20 installations should be notified they can update their existing analytics code to track click events that were being missed.
Additional documentation:
N/A