Analytics: move page views to its own endpoint #7739
Conversation
This allow us to cache the footer response and keep receiving page views.
stsewd
force-pushed
the
move-analytics
branch
from
5d391c1 to
0ef140a
Compare
| # Allow folks to hook onto the footer response for various information | ||
| # collection, or to modify the resp_data. | ||
| footer_response.send( | ||
| sender=None, | ||
| request=request, | ||
| context=context, | ||
| response_data=resp_data, | ||
| absolute_uri=self.request.GET.get('absolute_uri'), | ||
| ) | ||
|
|
There was a problem hiding this comment.
Shouldn't we keep sending this signal here?
We were only using it for pageviews here, but it could be useful for other things as well. Besides, it may have an impact downstream.
There was a problem hiding this comment.
Don't think this is useful, it also hides additional queries. The class can be extended already with OverrideSettginsObject, that's better than using signals for this case.
@readthedocs/core thoughts?
There was a problem hiding this comment.
My only worry is about custom installs that might be using it, but I don't expect it to be heavily used there. We plan to cache it in prod, but not all users will necessarily do that. I think I'm OK removing it, and we can re-add it if someone yells :)
| permission_classes = [IsAuthorizedToViewVersion] | ||
|
|
||
| def _get_project(self): | ||
| cache_key = '__cached_project' |
There was a problem hiding this comment.
We have this code in a few places now, we should probably move it to a mixin.
There was a problem hiding this comment.
I'll do another PR with that
| # Allow folks to hook onto the footer response for various information | ||
| # collection, or to modify the resp_data. | ||
| footer_response.send( | ||
| sender=None, | ||
| request=request, | ||
| context=context, | ||
| response_data=resp_data, | ||
| absolute_uri=self.request.GET.get('absolute_uri'), | ||
| ) | ||
|
|
There was a problem hiding this comment.
My only worry is about custom installs that might be using it, but I don't expect it to be heavily used there. We plan to cache it in prod, but not all users will necessarily do that. I think I'm OK removing it, and we can re-add it if someone yells :)
| @@ -427,7 +427,7 @@ def test_highest_version_without_tags(self): | |||
| class TestFooterPerformance(TestCase): | |||
| # The expected number of queries for generating the footer | |||
| # This shouldn't increase unless we modify the footer API | |||
| EXPECTED_QUERIES = 18 | |||
| EXPECTED_QUERIES = 14 | |||
Co-authored-by: Eric Holscher <[email protected]>
This allow us to cache the footer response and keep receiving page
views.
I have removed the analytics signals, since they were only used for this purpose, there are other PRs in -ext and -corp to follow this change.