-
Notifications
You must be signed in to change notification settings - Fork 61
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
Server changes needed for view time #418
Conversation
Note: these changes include a migration that adds a column to the Offer table. This should be run with extreme care in prod.
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.
Simple enough change. The migration definitely scares me, though maybe it won't be blocking, since it's nullable?
Co-authored-by: Eric Holscher <[email protected]>
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.
Changes look good 👍
Sidenote: are we currently using the send_to_analytics
function? I don't think we are -- might make sense to remove it.
@@ -722,6 +734,35 @@ def get_response(self, request, advertisement, publisher): | |||
return HttpResponseRedirect(url) | |||
|
|||
|
|||
class AdViewTimeProxyView(AdViewProxyView): |
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.
What are we using from the AdViewProxyView
that requires subclassing? Seems we're removing most of the logic?
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.
Mostly just getting the relevant offer, publisher, and ad.
We were at one point but I don't believe we are currently. |
This adds the server changes needed to account for view time sent from the client in readthedocs/ethical-ad-client#76.
Note: these changes include a migration that adds a column to the Offer table. This should be run with extreme care in prod.
Fixes #359 from a data capture perspective but the data still needs to be put into reports and such. We should probably collect data for a while before doing that.