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
Record opening and downloading resources #40076
Conversation
onClick={() => { | ||
this.recordResourceOpened(resource); | ||
}} |
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.
like with #40071, it doesn't really work to mix onClick functionality with standard links, particularly when the onClick is making an HTTP request
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.
Alright will make the same update here :)
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.
LGTM after addressing comments.
}) | ||
}, | ||
{includeUserId: true} | ||
); |
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.
if you've just got one thing in data json and you want it to be queryable, you could store it in data_int in addition / instead:
code-dot-org/apps/src/lib/util/firehose.js
Lines 12 to 23 in ed86d57
* Usage: | |
* firehoseClient.putRecord( | |
* { | |
* study: 'underwater basket weaving', // REQUIRED | |
* study_group: 'control', // OPTIONAL | |
* event: 'drowning', // REQUIRED | |
* data_int: 2 // OPTIONAL | |
* data_float: 0.31 // OPTIONAL | |
* data_string: 'code.org rocks' // OPTIONAL | |
* data_json: JSON.stringify(x) // OPTIONAL | |
* } | |
* ); |
my personal preference has been to store it in both places, so that the person writing the query can infer what the meaning of the data_int column is by seeing that it matches a named field in the data_json. since there is only one thing in data_json, this would let you proceed without further understanding of how this metric is going to be used.
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.
Done
LGTM |
Record firehose event when user clicks print pdf button on teacher lesson plan.
study: 'lesson-plan' or 'rollup-pages'
study_group: 'teacher-lesson-plan', 'student-lesson-plan', or 'resources-rollup'
event: 'download-resource' or 'open-resource'
data_json: includes resourceId
uuid included
Part of work for https://codedotorg.atlassian.net/browse/PLAT-339